diff --git a/CHANGELOG.md b/CHANGELOG.md index a4b281f0..a7118b0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,12 @@ ## Unreleased Changes * Added headless API for Studio companion plugins. ([#639]) +* Rojo now converts any line endings to LF, preventing spurious diffs when syncing Lua files on Windows ([#854]) +* Fixed Rojo plugin failing to connect when project contains certain unreadable properties ([#848]) * Added popout diff visualizer for table properties like Attributes and Tags ([#834]) * Updated Theme to use Studio colors ([#838]) * Added experimental setting for Auto Connect in playtests ([#840]) +* Fixed http error handling so Rojo can be used in Github Codespaces ([#847]) * Projects may now specify rules for syncing files as if they had a different file extension. ([#813]) This is specified via a new field on project files, `syncRules`: @@ -57,6 +60,10 @@ [#834]: https://github.com/rojo-rbx/rojo/pull/834 [#838]: https://github.com/rojo-rbx/rojo/pull/838 [#840]: https://github.com/rojo-rbx/rojo/pull/840 +[#847]: https://github.com/rojo-rbx/rojo/pull/847 +[#848]: https://github.com/rojo-rbx/rojo/pull/848 +[#854]: https://github.com/rojo-rbx/rojo/pull/854 + ## [7.4.0] - January 16, 2024 * Improved the visualization for array properties like Tags ([#829]) diff --git a/crates/memofs/CHANGELOG.md b/crates/memofs/CHANGELOG.md index c9961d0e..d678f8c9 100644 --- a/crates/memofs/CHANGELOG.md +++ b/crates/memofs/CHANGELOG.md @@ -2,8 +2,10 @@ ## Unreleased Changes * Changed `StdBackend` file watching component to use minimal recursive watches. [#830] +* Added `Vfs::read_to_string` and `Vfs::read_to_string_lf_normalized` [#854] [#830]: https://github.com/rojo-rbx/rojo/pull/830 +[#854]: https://github.com/rojo-rbx/rojo/pull/854 ## 0.2.0 (2021-08-23) * Updated to `crossbeam-channel` 0.5.1. diff --git a/crates/memofs/src/lib.rs b/crates/memofs/src/lib.rs index 744ca60f..6d41c853 100644 --- a/crates/memofs/src/lib.rs +++ b/crates/memofs/src/lib.rs @@ -22,9 +22,9 @@ mod noop_backend; mod snapshot; mod std_backend; -use std::io; use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex, MutexGuard}; +use std::{io, str}; pub use in_memory_fs::InMemoryFs; pub use noop_backend::NoopBackend; @@ -155,6 +155,24 @@ impl VfsInner { Ok(Arc::new(contents)) } + fn read_to_string>(&mut self, path: P) -> io::Result> { + let path = path.as_ref(); + let contents = self.backend.read(path)?; + + if self.watch_enabled { + self.backend.watch(path)?; + } + + let contents_str = str::from_utf8(&contents).map_err(|_| { + io::Error::new( + io::ErrorKind::InvalidData, + format!("File was not valid UTF-8: {}", path.display()), + ) + })?; + + Ok(Arc::new(contents_str.into())) + } + fn write, C: AsRef<[u8]>>(&mut self, path: P, contents: C) -> io::Result<()> { let path = path.as_ref(); let contents = contents.as_ref(); @@ -258,6 +276,33 @@ impl Vfs { self.inner.lock().unwrap().read(path) } + /// Read a file from the VFS (or from the underlying backend if it isn't + /// resident) into a string. + /// + /// Roughly equivalent to [`std::fs::read_to_string`][std::fs::read_to_string]. + /// + /// [std::fs::read_to_string]: https://doc.rust-lang.org/stable/std/fs/fn.read_to_string.html + #[inline] + pub fn read_to_string>(&self, path: P) -> io::Result> { + let path = path.as_ref(); + self.inner.lock().unwrap().read_to_string(path) + } + + /// Read a file from the VFS (or the underlying backend if it isn't + /// resident) into a string, and normalize its line endings to LF. + /// + /// Roughly equivalent to [`std::fs::read_to_string`][std::fs::read_to_string], but also performs + /// line ending normalization. + /// + /// [std::fs::read_to_string]: https://doc.rust-lang.org/stable/std/fs/fn.read_to_string.html + #[inline] + pub fn read_to_string_lf_normalized>(&self, path: P) -> io::Result> { + let path = path.as_ref(); + let contents = self.inner.lock().unwrap().read_to_string(path)?; + + Ok(contents.lines().collect::>().join("\n").into()) + } + /// Write a file to the VFS and the underlying backend. /// /// Roughly equivalent to [`std::fs::write`][std::fs::write]. diff --git a/plugin/http/Error.lua b/plugin/http/Error.lua index ed85fc67..f9e46b38 100644 --- a/plugin/http/Error.lua +++ b/plugin/http/Error.lua @@ -3,12 +3,12 @@ Error.__index = Error Error.Kind = { HttpNotEnabled = { - message = "Rojo requires HTTP access, which is not enabled.\n" .. - "Check your game settings, located in the 'Home' tab of Studio.", + message = "Rojo requires HTTP access, which is not enabled.\n" + .. "Check your game settings, located in the 'Home' tab of Studio.", }, ConnectFailed = { - message = "Couldn't connect to the Rojo server.\n" .. - "Make sure the server is running — use 'rojo serve' to run it!", + message = "Couldn't connect to the Rojo server.\n" + .. "Make sure the server is running — use 'rojo serve' to run it!", }, Timeout = { message = "HTTP request timed out.", @@ -63,4 +63,13 @@ function Error.fromRobloxErrorString(message) return Error.new(Error.Kind.Unknown, message) end +function Error.fromResponse(response) + local lower = (response.body or ""):lower() + if response.code == 408 or response.code == 504 or lower:find("timed? ?out") then + return Error.new(Error.Kind.Timeout) + end + + return Error.new(Error.Kind.Unknown, string.format("%s: %s", tostring(response.code), tostring(response.body))) +end + return Error diff --git a/plugin/http/init.lua b/plugin/http/init.lua index 686130ef..cac0ebc4 100644 --- a/plugin/http/init.lua +++ b/plugin/http/init.lua @@ -30,8 +30,13 @@ local function performRequest(requestParams) end) if success then - Log.trace("Request {} success, status code {}", requestId, response.StatusCode) - resolve(HttpResponse.fromRobloxResponse(response)) + Log.trace("Request {} success, response {:#?}", requestId, response) + local httpResponse = HttpResponse.fromRobloxResponse(response) + if httpResponse:isSuccess() then + resolve(httpResponse) + else + reject(HttpError.fromResponse(httpResponse)) + end else Log.trace("Request {} failure: {:?}", requestId, response) reject(HttpError.fromRobloxErrorString(response)) @@ -63,4 +68,4 @@ function Http.jsonDecode(source) return HttpService:JSONDecode(source) end -return Http \ No newline at end of file +return Http diff --git a/plugin/src/App/Components/StringDiffVisualizer/init.lua b/plugin/src/App/Components/StringDiffVisualizer/init.lua index 60eb49a3..e4203c21 100644 --- a/plugin/src/App/Components/StringDiffVisualizer/init.lua +++ b/plugin/src/App/Components/StringDiffVisualizer/init.lua @@ -9,6 +9,7 @@ local Log = require(Packages.Log) local Highlighter = require(Packages.Highlighter) local StringDiff = require(script:FindFirstChild("StringDiff")) +local Timer = require(Plugin.Timer) local Theme = require(Plugin.App.Theme) local CodeLabel = require(Plugin.App.Components.CodeLabel) @@ -74,6 +75,7 @@ function StringDiffVisualizer:calculateContentSize() end function StringDiffVisualizer:calculateDiffLines() + Timer.start("StringDiffVisualizer:calculateDiffLines") local oldString, newString = self.props.oldString, self.props.newString -- Diff the two texts @@ -133,6 +135,7 @@ function StringDiffVisualizer:calculateDiffLines() end end + Timer.stop() return add, remove end diff --git a/plugin/src/App/Components/TableDiffVisualizer/Array.lua b/plugin/src/App/Components/TableDiffVisualizer/Array.lua index 4fd56f3c..febd07e9 100644 --- a/plugin/src/App/Components/TableDiffVisualizer/Array.lua +++ b/plugin/src/App/Components/TableDiffVisualizer/Array.lua @@ -4,6 +4,7 @@ local Packages = Rojo.Packages local Roact = require(Packages.Roact) +local Timer = require(Plugin.Timer) local Assets = require(Plugin.Assets) local Theme = require(Plugin.App.Theme) @@ -21,6 +22,7 @@ function Array:init() end function Array:calculateDiff() + Timer.start("Array:calculateDiff") --[[ Find the indexes that are added or removed from the array, and display them side by side with gaps for the indexes that @@ -63,6 +65,7 @@ function Array:calculateDiff() j += 1 end + Timer.stop() return diff end diff --git a/plugin/src/App/Components/TableDiffVisualizer/Dictionary.lua b/plugin/src/App/Components/TableDiffVisualizer/Dictionary.lua index d0c32d96..033956c3 100644 --- a/plugin/src/App/Components/TableDiffVisualizer/Dictionary.lua +++ b/plugin/src/App/Components/TableDiffVisualizer/Dictionary.lua @@ -4,6 +4,7 @@ local Packages = Rojo.Packages local Roact = require(Packages.Roact) +local Timer = require(Plugin.Timer) local Assets = require(Plugin.Assets) local Theme = require(Plugin.App.Theme) @@ -21,6 +22,7 @@ function Dictionary:init() end function Dictionary:calculateDiff() + Timer.start("Dictionary:calculateDiff") local oldTable, newTable = self.props.oldTable or {}, self.props.newTable or {} -- Diff the two tables and find the added keys, removed keys, and changed keys @@ -59,6 +61,7 @@ function Dictionary:calculateDiff() return a.key < b.key end) + Timer.stop() return diff end diff --git a/plugin/src/App/StatusPages/Confirming.lua b/plugin/src/App/StatusPages/Confirming.lua index d3b6baa2..bd0fd0a1 100644 --- a/plugin/src/App/StatusPages/Confirming.lua +++ b/plugin/src/App/StatusPages/Confirming.lua @@ -4,6 +4,8 @@ local Packages = Rojo.Packages local Roact = require(Packages.Roact) +local Timer = require(Plugin.Timer) +local PatchTree = require(Plugin.PatchTree) local Settings = require(Plugin.Settings) local Theme = require(Plugin.App.Theme) local TextButton = require(Plugin.App.Components.TextButton) @@ -23,6 +25,7 @@ function ConfirmingPage:init() self.containerSize, self.setContainerSize = Roact.createBinding(Vector2.new(0, 0)) self:setState({ + patchTree = nil, showingStringDiff = false, oldString = "", newString = "", @@ -30,6 +33,28 @@ function ConfirmingPage:init() oldTable = {}, newTable = {}, }) + + if self.props.confirmData and self.props.confirmData.patch and self.props.confirmData.instanceMap then + self:buildPatchTree() + end +end + +function ConfirmingPage:didUpdate(prevProps) + if prevProps.confirmData ~= self.props.confirmData then + self:buildPatchTree() + end +end + +function ConfirmingPage:buildPatchTree() + Timer.start("ConfirmingPage:buildPatchTree") + self:setState({ + patchTree = PatchTree.build( + self.props.confirmData.patch, + self.props.confirmData.instanceMap, + { "Property", "Current", "Incoming" } + ), + }) + Timer.stop() end function ConfirmingPage:render() @@ -61,9 +86,7 @@ function ConfirmingPage:render() transparency = self.props.transparency, layoutOrder = 3, - changeListHeaders = { "Property", "Current", "Incoming" }, - patch = self.props.confirmData.patch, - instanceMap = self.props.confirmData.instanceMap, + patchTree = self.state.patchTree, showStringDiff = function(oldString: string, newString: string) self:setState({ diff --git a/plugin/src/App/StatusPages/Settings/Setting.lua b/plugin/src/App/StatusPages/Settings/Setting.lua index 74f04982..b75686f4 100644 --- a/plugin/src/App/StatusPages/Settings/Setting.lua +++ b/plugin/src/App/StatusPages/Settings/Setting.lua @@ -124,8 +124,14 @@ function Setting:render() BackgroundTransparency = 1, }, { Name = e("TextLabel", { - Text = (if self.props.experimental then '⚠ ' else "") - .. self.props.name, + Text = ( + if self.props.experimental + then '⚠ ' + elseif + self.props.developerDebug + then '⚑ ' -- Guru is the only font with the flag emoji + else "" + ) .. self.props.name, Font = Enum.Font.GothamBold, TextSize = 17, TextColor3 = theme.Setting.NameColor, @@ -140,8 +146,10 @@ function Setting:render() }), Description = e("TextLabel", { - Text = (if self.props.experimental then '[Experimental] ' else "") - .. self.props.description, + Text = (if self.props.experimental + then '[Experimental] ' + elseif self.props.developerDebug then '[Dev Debug] ' + else "") .. self.props.description, Font = Enum.Font.Gotham, LineHeight = 1.2, TextSize = 14, diff --git a/plugin/src/App/StatusPages/Settings/init.lua b/plugin/src/App/StatusPages/Settings/init.lua index ee5e72d5..399b536c 100644 --- a/plugin/src/App/StatusPages/Settings/init.lua +++ b/plugin/src/App/StatusPages/Settings/init.lua @@ -83,185 +83,198 @@ function SettingsPage:render() end return Theme.with(function(theme) - return e(ScrollingFrame, { - size = UDim2.new(1, 0, 1, 0), - contentSize = self.contentSize, - transparency = self.props.transparency, - }, { + return Roact.createFragment({ Navbar = e(Navbar, { onBack = self.props.onBack, transparency = self.props.transparency, layoutOrder = layoutIncrement(), }), - - ShowNotifications = e(Setting, { - id = "showNotifications", - name = "Show Notifications", - description = "Popup notifications in viewport", + Content = e(ScrollingFrame, { + size = UDim2.new(1, 0, 1, -47), + position = UDim2.new(0, 0, 0, 47), + contentSize = self.contentSize, transparency = self.props.transparency, - layoutOrder = layoutIncrement(), - }), - - SyncReminder = e(Setting, { - id = "syncReminder", - name = "Sync Reminder", - description = "Notify to sync when opening a place that has previously been synced", - transparency = self.props.transparency, - visible = Settings:getBinding("showNotifications"), - layoutOrder = layoutIncrement(), - }), - - ConfirmationBehavior = e(Setting, { - id = "confirmationBehavior", - name = "Confirmation Behavior", - description = "When to prompt for confirmation before syncing", - transparency = self.props.transparency, - layoutOrder = layoutIncrement(), - - options = confirmationBehaviors, - }), - - LargeChangesConfirmationThreshold = e(Setting, { - id = "largeChangesConfirmationThreshold", - name = "Confirmation Threshold", - description = "How many modified instances to be considered a large change", - transparency = self.props.transparency, - layoutOrder = layoutIncrement(), - visible = Settings:getBinding("confirmationBehavior"):map(function(value) - return value == "Large Changes" - end), - input = e(TextInput, { - size = UDim2.new(0, 40, 0, 28), - text = Settings:getBinding("largeChangesConfirmationThreshold"):map(function(value) - return tostring(value) - end), + }, { + ShowNotifications = e(Setting, { + id = "showNotifications", + name = "Show Notifications", + description = "Popup notifications in viewport", transparency = self.props.transparency, - enabled = true, - onEntered = function(text) - local number = tonumber(string.match(text, "%d+")) - if number then - Settings:set("largeChangesConfirmationThreshold", math.clamp(number, 1, 999)) - else - -- Force text back to last valid value - Settings:set( - "largeChangesConfirmationThreshold", - Settings:get("largeChangesConfirmationThreshold") - ) - end - end, + layoutOrder = layoutIncrement(), }), - }), - PlaySounds = e(Setting, { - id = "playSounds", - name = "Play Sounds", - description = "Toggle sound effects", - transparency = self.props.transparency, - layoutOrder = layoutIncrement(), - }), + SyncReminder = e(Setting, { + id = "syncReminder", + name = "Sync Reminder", + description = "Notify to sync when opening a place that has previously been synced", + transparency = self.props.transparency, + visible = Settings:getBinding("showNotifications"), + layoutOrder = layoutIncrement(), + }), - AutoConnectPlaytestServer = e(Setting, { - id = "autoConnectPlaytestServer", - name = "Auto Connect Playtest Server", - description = "Automatically connect game server to Rojo when playtesting while connected in Edit", - experimental = true, - transparency = self.props.transparency, - layoutOrder = layoutIncrement(), - }), + ConfirmationBehavior = e(Setting, { + id = "confirmationBehavior", + name = "Confirmation Behavior", + description = "When to prompt for confirmation before syncing", + transparency = self.props.transparency, + layoutOrder = layoutIncrement(), - Permissions = e(Setting, { - name = "Third Party Permissions", - description = "Manage permissions for third party plugins", - transparency = self.props.transparency, - layoutOrder = 6, - input = e("TextButton", { - Text = "", - BackgroundTransparency = 1, - Size = UDim2.fromOffset(28, 28), - Position = UDim2.fromScale(1, 0.5), - AnchorPoint = Vector2.new(1, 0.5), + options = confirmationBehaviors, + }), - [Roact.Event.Activated] = function() - self.props.onNavigatePermissions() - end, - }, { - Button = e(SlicedImage, { - slice = Assets.Slices.RoundedBackground, - color = theme.Checkbox.Active.BackgroundColor, + LargeChangesConfirmationThreshold = e(Setting, { + id = "largeChangesConfirmationThreshold", + name = "Confirmation Threshold", + description = "How many modified instances to be considered a large change", + transparency = self.props.transparency, + layoutOrder = layoutIncrement(), + visible = Settings:getBinding("confirmationBehavior"):map(function(value) + return value == "Large Changes" + end), + input = e(TextInput, { + size = UDim2.new(0, 40, 0, 28), + text = Settings:getBinding("largeChangesConfirmationThreshold"):map(function(value) + return tostring(value) + end), transparency = self.props.transparency, - size = UDim2.new(1, 0, 1, 0), + enabled = true, + onEntered = function(text) + local number = tonumber(string.match(text, "%d+")) + if number then + Settings:set("largeChangesConfirmationThreshold", math.clamp(number, 1, 999)) + else + -- Force text back to last valid value + Settings:set( + "largeChangesConfirmationThreshold", + Settings:get("largeChangesConfirmationThreshold") + ) + end + end, + }), + }), + + PlaySounds = e(Setting, { + id = "playSounds", + name = "Play Sounds", + description = "Toggle sound effects", + transparency = self.props.transparency, + layoutOrder = layoutIncrement(), + }), + + AutoConnectPlaytestServer = e(Setting, { + id = "autoConnectPlaytestServer", + name = "Auto Connect Playtest Server", + description = "Automatically connect game server to Rojo when playtesting while connected in Edit", + experimental = true, + transparency = self.props.transparency, + layoutOrder = layoutIncrement(), + }), + + Permissions = e(Setting, { + name = "Third Party Permissions", + description = "Manage permissions for third party plugins", + transparency = self.props.transparency, + layoutOrder = layoutIncrement(), + input = e("TextButton", { + Text = "", + BackgroundTransparency = 1, + Size = UDim2.fromOffset(28, 28), + Position = UDim2.fromScale(1, 0.5), + AnchorPoint = Vector2.new(1, 0.5), + + [Roact.Event.Activated] = function() + self.props.onNavigatePermissions() + end, }, { - Icon = e("ImageLabel", { - Image = Assets.Images.Icons.Expand, - ImageColor3 = theme.Checkbox.Active.IconColor, - ImageTransparency = self.props.transparency, + Button = e(SlicedImage, { + slice = Assets.Slices.RoundedBackground, + color = theme.Checkbox.Active.BackgroundColor, + transparency = self.props.transparency, + size = UDim2.new(1, 0, 1, 0), + }, { + Icon = e("ImageLabel", { + Image = Assets.Images.Icons.Expand, + ImageColor3 = theme.Checkbox.Active.IconColor, + ImageTransparency = self.props.transparency, - Size = UDim2.new(0, 16, 0, 16), - Position = UDim2.new(0.5, 0, 0.5, 0), - AnchorPoint = Vector2.new(0.5, 0.5), + Size = UDim2.new(0, 16, 0, 16), + Position = UDim2.new(0.5, 0, 0.5, 0), + AnchorPoint = Vector2.new(0.5, 0.5), - BackgroundTransparency = 1, + BackgroundTransparency = 1, + }), }), }), }), - }), - OpenScriptsExternally = e(Setting, { - id = "openScriptsExternally", - name = "Open Scripts Externally", - description = "Attempt to open scripts in an external editor", - locked = self.props.syncActive, - experimental = true, - transparency = self.props.transparency, - layoutOrder = layoutIncrement(), - }), + OpenScriptsExternally = e(Setting, { + id = "openScriptsExternally", + name = "Open Scripts Externally", + description = "Attempt to open scripts in an external editor", + locked = self.props.syncActive, + experimental = true, + transparency = self.props.transparency, + layoutOrder = layoutIncrement(), + }), - TwoWaySync = e(Setting, { - id = "twoWaySync", - name = "Two-Way Sync", - description = "Editing files in Studio will sync them into the filesystem", - locked = self.props.syncActive, - experimental = true, - transparency = self.props.transparency, - layoutOrder = layoutIncrement(), - }), + TwoWaySync = e(Setting, { + id = "twoWaySync", + name = "Two-Way Sync", + description = "Editing files in Studio will sync them into the filesystem", + locked = self.props.syncActive, + experimental = true, + transparency = self.props.transparency, + layoutOrder = layoutIncrement(), + }), - LogLevel = e(Setting, { - id = "logLevel", - name = "Log Level", - description = "Plugin output verbosity level", - transparency = self.props.transparency, - layoutOrder = layoutIncrement(), + LogLevel = e(Setting, { + id = "logLevel", + name = "Log Level", + description = "Plugin output verbosity level", + developerDebug = true, + transparency = self.props.transparency, + layoutOrder = layoutIncrement(), - options = invertedLevels, - showReset = Settings:getBinding("logLevel"):map(function(value) - return value ~= "Info" - end), - onReset = function() - Settings:set("logLevel", "Info") - end, - }), + options = invertedLevels, + showReset = Settings:getBinding("logLevel"):map(function(value) + return value ~= "Info" + end), + onReset = function() + Settings:set("logLevel", "Info") + end, + }), - TypecheckingEnabled = e(Setting, { - id = "typecheckingEnabled", - name = "Typechecking", - description = "Toggle typechecking on the API surface", - transparency = self.props.transparency, - layoutOrder = layoutIncrement(), - }), + TypecheckingEnabled = e(Setting, { + id = "typecheckingEnabled", + name = "Typechecking", + description = "Toggle typechecking on the API surface", + developerDebug = true, + transparency = self.props.transparency, + layoutOrder = layoutIncrement(), + }), - Layout = e("UIListLayout", { - FillDirection = Enum.FillDirection.Vertical, - SortOrder = Enum.SortOrder.LayoutOrder, + TimingLogsEnabled = e(Setting, { + id = "timingLogsEnabled", + name = "Timing Logs", + description = "Toggle logging timing of internal actions for benchmarking Rojo performance", + developerDebug = true, + transparency = self.props.transparency, + layoutOrder = layoutIncrement(), + }), - [Roact.Change.AbsoluteContentSize] = function(object) - self.setContentSize(object.AbsoluteContentSize) - end, - }), + Layout = e("UIListLayout", { + FillDirection = Enum.FillDirection.Vertical, + SortOrder = Enum.SortOrder.LayoutOrder, - Padding = e("UIPadding", { - PaddingLeft = UDim.new(0, 20), - PaddingRight = UDim.new(0, 20), + [Roact.Change.AbsoluteContentSize] = function(object) + self.setContentSize(object.AbsoluteContentSize) + end, + }), + + Padding = e("UIPadding", { + PaddingLeft = UDim.new(0, 20), + PaddingRight = UDim.new(0, 20), + }), }), }) end) diff --git a/plugin/src/PatchTree.lua b/plugin/src/PatchTree.lua index e469e211..d0d99e37 100644 --- a/plugin/src/PatchTree.lua +++ b/plugin/src/PatchTree.lua @@ -11,6 +11,7 @@ local Packages = Rojo.Packages local Log = require(Packages.Log) +local Timer = require(Plugin.Timer) local Types = require(Plugin.Types) local decodeValue = require(Plugin.Reconciler.decodeValue) local getProperty = require(Plugin.Reconciler.getProperty) @@ -122,6 +123,7 @@ end -- props must contain id, and cannot contain children or parentId -- other than those three, it can hold anything function Tree:addNode(parent, props) + Timer.start("Tree:addNode") assert(props.id, "props must contain id") parent = parent or "ROOT" @@ -132,6 +134,7 @@ function Tree:addNode(parent, props) for k, v in props do node[k] = v end + Timer.stop() return node end @@ -142,22 +145,26 @@ function Tree:addNode(parent, props) local parentNode = self:getNode(parent) if not parentNode then Log.warn("Failed to create node since parent doesnt exist: {}, {}", parent, props) + Timer.stop() return end parentNode.children[node.id] = node self.idToNode[node.id] = node + Timer.stop() return node end -- Given a list of ancestor ids in descending order, builds the nodes for them -- using the patch and instanceMap info function Tree:buildAncestryNodes(previousId: string?, ancestryIds: { string }, patch, instanceMap) + Timer.start("Tree:buildAncestryNodes") -- Build nodes for ancestry by going up the tree previousId = previousId or "ROOT" - for _, ancestorId in ancestryIds do + for i = #ancestryIds, 1, -1 do + local ancestorId = ancestryIds[i] local value = instanceMap.fromIds[ancestorId] or patch.added[ancestorId] if not value then Log.warn("Failed to find ancestor object for " .. ancestorId) @@ -171,6 +178,8 @@ function Tree:buildAncestryNodes(previousId: string?, ancestryIds: { string }, p }) previousId = ancestorId end + + Timer.stop() end local PatchTree = {} @@ -178,10 +187,12 @@ local PatchTree = {} -- Builds a new tree from a patch and instanceMap -- uses changeListHeaders in node.changeList function PatchTree.build(patch, instanceMap, changeListHeaders) + Timer.start("PatchTree.build") local tree = Tree.new() local knownAncestors = {} + Timer.start("patch.updated") for _, change in patch.updated do local instance = instanceMap.fromIds[change.id] if not instance then @@ -189,7 +200,7 @@ function PatchTree.build(patch, instanceMap, changeListHeaders) end -- Gather ancestors from existing DOM - local ancestryIds = {} + local ancestryIds, ancestryIndex = {}, 0 local parentObject = instance.Parent local parentId = instanceMap.fromInstances[parentObject] local previousId = nil @@ -200,7 +211,8 @@ function PatchTree.build(patch, instanceMap, changeListHeaders) break end - table.insert(ancestryIds, 1, parentId) + ancestryIndex += 1 + ancestryIds[ancestryIndex] = parentId knownAncestors[parentId] = true parentObject = parentObject.Parent parentId = instanceMap.fromInstances[parentObject] @@ -213,11 +225,38 @@ function PatchTree.build(patch, instanceMap, changeListHeaders) if next(change.changedProperties) or change.changedName then changeList = {} - local hintBuffer, i = {}, 0 + local hintBuffer, hintBufferSize, hintOverflow = table.create(3), 0, 0 + local changeIndex = 0 local function addProp(prop: string, current: any?, incoming: any?, metadata: any?) - i += 1 - hintBuffer[i] = prop - changeList[i] = { prop, current, incoming, metadata } + changeIndex += 1 + changeList[changeIndex] = { prop, current, incoming, metadata } + + if hintBufferSize < 3 then + hintBufferSize += 1 + hintBuffer[hintBufferSize] = prop + return + end + + -- We only want to have 3 hints + -- to keep it deterministic, we sort them alphabetically + + -- Either this prop overflows, or it makes another one move to overflow + hintOverflow += 1 + + -- Shortcut for the common case + if hintBuffer[3] <= prop then + -- This prop is below the last hint, no need to insert + return + end + + -- Find the first available spot + for i, hintItem in hintBuffer do + if prop < hintItem then + -- This prop is before the currently selected hint, + -- so take its place and then continue to find a spot for the old hint + hintBuffer[i], prop = prop, hintBuffer[i] + end + end end -- Gather the changes @@ -238,18 +277,7 @@ function PatchTree.build(patch, instanceMap, changeListHeaders) end -- Finalize detail values - - -- Trim hint to top 3 - table.sort(hintBuffer) - if #hintBuffer > 3 then - hintBuffer = { - hintBuffer[1], - hintBuffer[2], - hintBuffer[3], - i - 3 .. " more", - } - end - hint = table.concat(hintBuffer, ", ") + hint = table.concat(hintBuffer, ", ") .. (if hintOverflow == 0 then "" else ", " .. hintOverflow .. " more") -- Sort changes and add header table.sort(changeList, function(a, b) @@ -269,7 +297,9 @@ function PatchTree.build(patch, instanceMap, changeListHeaders) changeList = changeList, }) end + Timer.stop() + Timer.start("patch.removed") for _, idOrInstance in patch.removed do local instance = if Types.RbxId(idOrInstance) then instanceMap.fromIds[idOrInstance] else idOrInstance if not instance then @@ -311,7 +341,9 @@ function PatchTree.build(patch, instanceMap, changeListHeaders) instance = instance, }) end + Timer.stop() + Timer.start("patch.added") for id, change in patch.added do -- Gather ancestors from existing DOM or future additions local ancestryIds = {} @@ -350,32 +382,47 @@ function PatchTree.build(patch, instanceMap, changeListHeaders) if next(change.Properties) then changeList = {} - local hintBuffer, i = {}, 0 - for prop, incoming in change.Properties do - i += 1 - hintBuffer[i] = prop + local hintBuffer, hintBufferSize, hintOverflow = table.create(3), 0, 0 + local changeIndex = 0 + local function addProp(prop: string, incoming: any) + changeIndex += 1 + changeList[changeIndex] = { prop, "N/A", incoming } - local success, incomingValue = decodeValue(incoming, instanceMap) - if success then - table.insert(changeList, { prop, "N/A", incomingValue }) - else - table.insert(changeList, { prop, "N/A", select(2, next(incoming)) }) + if hintBufferSize < 3 then + hintBufferSize += 1 + hintBuffer[hintBufferSize] = prop + return + end + + -- We only want to have 3 hints + -- to keep it deterministic, we sort them alphabetically + + -- Either this prop overflows, or it makes another one move to overflow + hintOverflow += 1 + + -- Shortcut for the common case + if hintBuffer[3] <= prop then + -- This prop is below the last hint, no need to insert + return + end + + -- Find the first available spot + for i, hintItem in hintBuffer do + if prop < hintItem then + -- This prop is before the currently selected hint, + -- so take its place and then continue to find a spot for the old hint + hintBuffer[i], prop = prop, hintBuffer[i] + end end end - -- Finalize detail values - - -- Trim hint to top 3 - table.sort(hintBuffer) - if #hintBuffer > 3 then - hintBuffer = { - hintBuffer[1], - hintBuffer[2], - hintBuffer[3], - i - 3 .. " more", - } + for prop, incoming in change.Properties do + local success, incomingValue = decodeValue(incoming, instanceMap) + addProp(prop, if success then incomingValue else select(2, next(incoming))) end - hint = table.concat(hintBuffer, ", ") + + -- Finalize detail values + hint = table.concat(hintBuffer, ", ") .. (if hintOverflow == 0 then "" else ", " .. hintOverflow .. " more") -- Sort changes and add header table.sort(changeList, function(a, b) @@ -395,35 +442,27 @@ function PatchTree.build(patch, instanceMap, changeListHeaders) instance = instanceMap.fromIds[id], }) end + Timer.stop() + Timer.stop() return tree end --- Creates a deep copy of a tree for immutability purposes in Roact -function PatchTree.clone(tree) - if not tree then - return - end - - local newTree = Tree.new() - tree:forEach(function(node) - newTree:addNode(node.parentId, table.clone(node)) - end) - - return newTree -end - -- Updates the metadata of a tree with the unapplied patch and currently existing instances -- Builds a new tree from the data if one isn't provided -- Always returns a new tree for immutability purposes in Roact function PatchTree.updateMetadata(tree, patch, instanceMap, unappliedPatch) + Timer.start("PatchTree.updateMetadata") if tree then - tree = PatchTree.clone(tree) + -- A shallow copy is enough for our purposes here since we really only need a new top-level object + -- for immutable comparison checks in Roact + tree = table.clone(tree) else tree = PatchTree.build(patch, instanceMap) end -- Update isWarning metadata + Timer.start("isWarning") for _, failedChange in unappliedPatch.updated do local node = tree:getNode(failedChange.id) if not node then @@ -492,8 +531,10 @@ function PatchTree.updateMetadata(tree, patch, instanceMap, unappliedPatch) node.isWarning = true Log.trace("Marked node as warning: {} {}", node.id, node.name) end + Timer.stop() -- Update if instances exist + Timer.start("instanceAncestry") tree:forEach(function(node) if node.instance then if node.instance.Parent == nil and node.instance ~= game then @@ -509,7 +550,9 @@ function PatchTree.updateMetadata(tree, patch, instanceMap, unappliedPatch) end end end) + Timer.stop() + Timer.stop() return tree end diff --git a/plugin/src/Reconciler/diff.lua b/plugin/src/Reconciler/diff.lua index e13b05c8..c87f899c 100644 --- a/plugin/src/Reconciler/diff.lua +++ b/plugin/src/Reconciler/diff.lua @@ -177,10 +177,8 @@ local function diff(instanceMap, virtualInstances, rootId) if err.kind == Error.UnknownProperty then Log.trace("Skipping unknown property {}.{}", err.details.className, err.details.propertyName) - elseif err.kind == Error.UnreadableProperty then - Log.trace("Skipping unreadable property {}.{}", err.details.className, err.details.propertyName) else - return false, err + Log.trace("Skipping unreadable property {}.{}", err.details.className, err.details.propertyName) end end end diff --git a/plugin/src/Reconciler/init.lua b/plugin/src/Reconciler/init.lua index 12945782..df7cd92f 100644 --- a/plugin/src/Reconciler/init.lua +++ b/plugin/src/Reconciler/init.lua @@ -3,9 +3,14 @@ and mutating the Roblox DOM. ]] -local Packages = script.Parent.Parent.Packages +local Rojo = script:FindFirstAncestor("Rojo") +local Plugin = Rojo.Plugin +local Packages = Rojo.Packages + local Log = require(Packages.Log) +local Timer = require(Plugin.Timer) + local applyPatch = require(script.applyPatch) local hydrate = require(script.hydrate) local diff = require(script.diff) @@ -57,31 +62,55 @@ function Reconciler:hookPostcommit(callback: (patch: any, instanceMap: any, unap end function Reconciler:applyPatch(patch) + Timer.start("Reconciler:applyPatch") + + Timer.start("precommitCallbacks") + -- Precommit callbacks must be serial in order to obey the contract that + -- they execute before commit for _, callback in self.__precommitCallbacks do local success, err = pcall(callback, patch, self.__instanceMap) if not success then Log.warn("Precommit hook errored: {}", err) end end + Timer.stop() + Timer.start("apply") local unappliedPatch = applyPatch(self.__instanceMap, patch) + Timer.stop() + Timer.start("postcommitCallbacks") + -- Postcommit callbacks can be called with spawn since regardless of firing order, they are + -- guaranteed to be called after the commit for _, callback in self.__postcommitCallbacks do - local success, err = pcall(callback, patch, self.__instanceMap, unappliedPatch) - if not success then - Log.warn("Postcommit hook errored: {}", err) - end + task.spawn(function() + local success, err = pcall(callback, patch, self.__instanceMap, unappliedPatch) + if not success then + Log.warn("Postcommit hook errored: {}", err) + end + end) end + Timer.stop() + + Timer.stop() return unappliedPatch end function Reconciler:hydrate(virtualInstances, rootId, rootInstance) - return hydrate(self.__instanceMap, virtualInstances, rootId, rootInstance) + Timer.start("Reconciler:hydrate") + local result = hydrate(self.__instanceMap, virtualInstances, rootId, rootInstance) + Timer.stop() + + return result end function Reconciler:diff(virtualInstances, rootId) - return diff(self.__instanceMap, virtualInstances, rootId) + Timer.start("Reconciler:diff") + local success, result = diff(self.__instanceMap, virtualInstances, rootId) + Timer.stop() + + return success, result end return Reconciler diff --git a/plugin/src/Settings.lua b/plugin/src/Settings.lua index 8389000e..2b1c121d 100644 --- a/plugin/src/Settings.lua +++ b/plugin/src/Settings.lua @@ -20,6 +20,7 @@ local defaultSettings = { playSounds = true, typecheckingEnabled = false, logLevel = "Info", + timingLogsEnabled = false, priorEndpoints = {}, apiPermissions = {}, } diff --git a/plugin/src/Timer.lua b/plugin/src/Timer.lua new file mode 100644 index 00000000..9a5bbbd0 --- /dev/null +++ b/plugin/src/Timer.lua @@ -0,0 +1,57 @@ +local Settings = require(script.Parent.Settings) + +local clock = os.clock + +local Timer = { + _entries = {}, +} + +function Timer._start(label) + local start = clock() + if not label then + error("[Rojo-Timer] Timer.start: label is required", 2) + return + end + + table.insert(Timer._entries, { label, start }) +end + +function Timer._stop() + local stop = clock() + + local entry = table.remove(Timer._entries) + if not entry then + error("[Rojo-Timer] Timer.stop: no label to stop", 2) + return + end + + local label = entry[1] + if #Timer._entries > 0 then + local priorLabels = {} + for _, priorEntry in ipairs(Timer._entries) do + table.insert(priorLabels, priorEntry[1]) + end + label = table.concat(priorLabels, "/") .. "/" .. label + end + + local start = entry[2] + local duration = stop - start + print(string.format("[Rojo-Timer] %s took %.3f ms", label, duration * 1000)) +end + +-- Replace functions with no-op if not in debug mode +local function no_op() end +local function setFunctions(enabled) + if enabled then + Timer.start = Timer._start + Timer.stop = Timer._stop + else + Timer.start = no_op + Timer.stop = no_op + end +end + +Settings:onChanged("timingLogsEnabled", setFunctions) +setFunctions(Settings:get("timingLogsEnabled")) + +return Timer diff --git a/src/snapshot_middleware/lua.rs b/src/snapshot_middleware/lua.rs index ab7f15a1..3f8d26ad 100644 --- a/src/snapshot_middleware/lua.rs +++ b/src/snapshot_middleware/lua.rs @@ -1,6 +1,5 @@ use std::{collections::HashMap, path::Path, str}; -use anyhow::Context; use memofs::{IoResultExt, Vfs}; use rbx_dom_weak::types::Enum; @@ -40,10 +39,8 @@ pub fn snapshot_lua( (_, ScriptType::Module) => ("ModuleScript", None), }; - let contents = vfs.read(path)?; - let contents_str = str::from_utf8(&contents) - .with_context(|| format!("File was not valid UTF-8: {}", path.display()))? - .to_owned(); + let contents = vfs.read_to_string_lf_normalized(path)?; + let contents_str = contents.as_str(); let mut properties = HashMap::with_capacity(2); properties.insert("Source".to_owned(), contents_str.into()); diff --git a/src/snapshot_middleware/txt.rs b/src/snapshot_middleware/txt.rs index 207243f9..a69b14d1 100644 --- a/src/snapshot_middleware/txt.rs +++ b/src/snapshot_middleware/txt.rs @@ -1,6 +1,5 @@ use std::{path::Path, str}; -use anyhow::Context; use maplit::hashmap; use memofs::{IoResultExt, Vfs}; @@ -14,10 +13,8 @@ pub fn snapshot_txt( path: &Path, name: &str, ) -> anyhow::Result> { - let contents = vfs.read(path)?; - let contents_str = str::from_utf8(&contents) - .with_context(|| format!("File was not valid UTF-8: {}", path.display()))? - .to_owned(); + let contents = vfs.read_to_string(path)?; + let contents_str = contents.as_str(); let properties = hashmap! { "Value".to_owned() => contents_str.into(),