From 54e63d88d4d538936bfb0b55d87dc20bc84ec36a Mon Sep 17 00:00:00 2001 From: boatbomber Date: Thu, 6 Nov 2025 00:06:42 -0800 Subject: [PATCH] Slightly improve initial sync hangs (#1140) --- CHANGELOG.md | 2 ++ plugin/src/App/StatusPages/Confirming.lua | 27 +---------------- plugin/src/App/StatusPages/Connecting.lua | 36 +++++++++++++++++++---- plugin/src/App/init.lua | 17 +++++++++-- plugin/src/PatchTree.lua | 26 +++++++++++----- plugin/src/ServeSession.lua | 14 +++++++++ 6 files changed, 81 insertions(+), 41 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6981303a..7a79b193 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,11 +33,13 @@ Making a new release? Simply add the new header with the version and date undern * Fixed a bug where the last sync timestamp was not updating correctly in the plugin ([#1132]) * Improved the reliability of sync replacements by adding better error handling and recovery ([#1135]) +* Small improvements to stability when syncing massive projects ([#1140]) * Added support for JSON comments and trailing commas in project, meta, and model json files ([#1144]) * Added `sourcemap.json` into the default `.gitignore` files ([#1145]) [#1132]: https://github.com/rojo-rbx/rojo/pull/1132 [#1135]: https://github.com/rojo-rbx/rojo/pull/1135 +[#1140]: https://github.com/rojo-rbx/rojo/pull/1140 [#1144]: https://github.com/rojo-rbx/rojo/pull/1144 [#1145]: https://github.com/rojo-rbx/rojo/pull/1145 diff --git a/plugin/src/App/StatusPages/Confirming.lua b/plugin/src/App/StatusPages/Confirming.lua index 3f3958a4..a334a83c 100644 --- a/plugin/src/App/StatusPages/Confirming.lua +++ b/plugin/src/App/StatusPages/Confirming.lua @@ -4,8 +4,6 @@ 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) @@ -24,7 +22,6 @@ function ConfirmingPage:init() self.containerSize, self.setContainerSize = Roact.createBinding(Vector2.new(0, 0)) self:setState({ - patchTree = nil, showingStringDiff = false, oldString = "", newString = "", @@ -32,28 +29,6 @@ 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() @@ -79,7 +54,7 @@ function ConfirmingPage:render() transparency = self.props.transparency, layoutOrder = 3, - patchTree = self.state.patchTree, + patchTree = self.props.patchTree, showStringDiff = function(oldString: string, newString: string) self:setState({ diff --git a/plugin/src/App/StatusPages/Connecting.lua b/plugin/src/App/StatusPages/Connecting.lua index b32b37bd..c3059a36 100644 --- a/plugin/src/App/StatusPages/Connecting.lua +++ b/plugin/src/App/StatusPages/Connecting.lua @@ -4,6 +4,8 @@ local Packages = Rojo.Packages local Roact = require(Packages.Roact) +local Theme = require(Plugin.App.Theme) + local Spinner = require(Plugin.App.Components.Spinner) local e = Roact.createElement @@ -11,11 +13,35 @@ local e = Roact.createElement local ConnectingPage = Roact.Component:extend("ConnectingPage") function ConnectingPage:render() - return e(Spinner, { - position = UDim2.new(0.5, 0, 0.5, 0), - anchorPoint = Vector2.new(0.5, 0.5), - transparency = self.props.transparency, - }) + return Theme.with(function(theme) + return e("Frame", { + Size = UDim2.new(1, 0, 1, 0), + BackgroundTransparency = 1, + }, { + Spinner = e(Spinner, { + position = UDim2.new(0.5, 0, 0.5, 0), + anchorPoint = Vector2.new(0.5, 0.5), + transparency = self.props.transparency, + }), + Text = if type(self.props.text) == "string" and #self.props.text > 0 + then e("TextLabel", { + Text = self.props.text, + Position = UDim2.new(0.5, 0, 0.5, 30), + Size = UDim2.new(1, -40, 0.5, -40), + AnchorPoint = Vector2.new(0.5, 0), + TextXAlignment = Enum.TextXAlignment.Center, + TextYAlignment = Enum.TextYAlignment.Top, + RichText = true, + FontFace = theme.Font.Thin, + TextSize = theme.TextSize.Medium, + TextColor3 = theme.SubTextColor, + TextTruncate = Enum.TextTruncate.AtEnd, + TextTransparency = self.props.transparency, + BackgroundTransparency = 1, + }) + else nil, + }) + end) end return ConnectingPage diff --git a/plugin/src/App/init.lua b/plugin/src/App/init.lua index f38596dd..07203eef 100644 --- a/plugin/src/App/init.lua +++ b/plugin/src/App/init.lua @@ -595,6 +595,12 @@ function App:startSession() twoWaySync = Settings:get("twoWaySync"), }) + serveSession:setUpdateLoadingTextCallback(function(text: string) + self:setState({ + connectingText = text, + }) + end) + self.cleanupPrecommit = serveSession:hookPrecommit(function(patch, instanceMap) -- Build new tree for patch self:setState({ @@ -759,11 +765,13 @@ function App:startSession() end end + self:setState({ + connectingText = "Computing diff view...", + }) self:setState({ appStatus = AppStatus.Confirming, + patchTree = PatchTree.build(patch, instanceMap, { "Property", "Current", "Incoming" }), confirmData = { - instanceMap = instanceMap, - patch = patch, serverInfo = serverInfo, }, toolbarIcon = Assets.Images.PluginButton, @@ -874,6 +882,7 @@ function App:render() ConfirmingPage = createPageElement(AppStatus.Confirming, { confirmData = self.state.confirmData, + patchTree = self.state.patchTree, createPopup = not self.state.guiEnabled, onAbort = function() @@ -887,7 +896,9 @@ function App:render() end, }), - Connecting = createPageElement(AppStatus.Connecting), + Connecting = createPageElement(AppStatus.Connecting, { + text = self.state.connectingText, + }), Connected = createPageElement(AppStatus.Connected, { projectName = self.state.projectName, diff --git a/plugin/src/PatchTree.lua b/plugin/src/PatchTree.lua index c1c5f91c..37deae11 100644 --- a/plugin/src/PatchTree.lua +++ b/plugin/src/PatchTree.lua @@ -16,6 +16,14 @@ local Types = require(Plugin.Types) local decodeValue = require(Plugin.Reconciler.decodeValue) local getProperty = require(Plugin.Reconciler.getProperty) +local function yieldIfNeeded(clock) + if os.clock() - clock > 1 / 20 then + task.wait() + return os.clock() + end + return clock +end + local function alphabeticalNext(t, state) -- Equivalent of the next function, but returns the keys in the alphabetic -- order of node names. We use a temporary ordered key table that is stored in the @@ -132,7 +140,6 @@ 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" @@ -143,7 +150,6 @@ function Tree:addNode(parent, props) for k, v in props do node[k] = v end - Timer.stop() return node end @@ -154,25 +160,25 @@ 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") + local clock = os.clock() -- Build nodes for ancestry by going up the tree previousId = previousId or "ROOT" for _, ancestorId in ancestryIds do + clock = yieldIfNeeded(clock) + local value = instanceMap.fromIds[ancestorId] or patch.added[ancestorId] if not value then Log.warn("Failed to find ancestor object for " .. ancestorId) @@ -186,8 +192,6 @@ function Tree:buildAncestryNodes(previousId: string?, ancestryIds: { string }, p }) previousId = ancestorId end - - Timer.stop() end local PatchTree = {} @@ -196,12 +200,16 @@ local PatchTree = {} -- uses changeListHeaders in node.changeList function PatchTree.build(patch, instanceMap, changeListHeaders) Timer.start("PatchTree.build") + local clock = os.clock() + local tree = Tree.new() local knownAncestors = {} Timer.start("patch.updated") for _, change in patch.updated do + clock = yieldIfNeeded(clock) + local instance = instanceMap.fromIds[change.id] if not instance then continue @@ -281,6 +289,8 @@ function PatchTree.build(patch, instanceMap, changeListHeaders) Timer.start("patch.removed") for _, idOrInstance in patch.removed do + clock = yieldIfNeeded(clock) + local instance = if Types.RbxId(idOrInstance) then instanceMap.fromIds[idOrInstance] else idOrInstance if not instance then -- If we're viewing a past patch, the instance is already removed @@ -325,6 +335,8 @@ function PatchTree.build(patch, instanceMap, changeListHeaders) Timer.start("patch.added") for id, change in patch.added do + clock = yieldIfNeeded(clock) + -- Gather ancestors from existing DOM or future additions local ancestryIds = {} local parentId = change.Parent diff --git a/plugin/src/ServeSession.lua b/plugin/src/ServeSession.lua index 749c9188..47ba2bb7 100644 --- a/plugin/src/ServeSession.lua +++ b/plugin/src/ServeSession.lua @@ -107,6 +107,7 @@ function ServeSession.new(options) __connections = connections, __precommitCallbacks = {}, __postcommitCallbacks = {}, + __updateLoadingText = function() end, } setmetatable(self, ServeSession) @@ -137,6 +138,14 @@ function ServeSession:setConfirmCallback(callback) self.__userConfirmCallback = callback end +function ServeSession:setUpdateLoadingTextCallback(callback) + self.__updateLoadingText = callback +end + +function ServeSession:setLoadingText(text: string) + self.__updateLoadingText(text) +end + --[=[ Hooks a function to run before patch application. The provided function is called with the incoming patch and an InstanceMap @@ -181,11 +190,14 @@ end function ServeSession:start() self:__setStatus(Status.Connecting) + self:setLoadingText("Connecting to server...") self.__apiContext :connect() :andThen(function(serverInfo) + self:setLoadingText("Loading initial data from server...") return self:__initialSync(serverInfo):andThen(function() + self:setLoadingText("Starting sync loop...") self:__setStatus(Status.Connected, serverInfo.projectName) self:__applyGameAndPlaceId(serverInfo) @@ -449,11 +461,13 @@ function ServeSession:__initialSync(serverInfo) -- For any instances that line up with the Rojo server's view, start -- tracking them in the reconciler. Log.trace("Matching existing Roblox instances to Rojo IDs") + self:setLoadingText("Hydrating instance map...") self.__reconciler:hydrate(readResponseBody.instances, serverInfo.rootInstanceId, game) -- Calculate the initial patch to apply to the DataModel to catch us -- up to what Rojo thinks the place should look like. Log.trace("Computing changes that plugin needs to make to catch up to server...") + self:setLoadingText("Finding differences between server and Studio...") local success, catchUpPatch = self.__reconciler:diff(readResponseBody.instances, serverInfo.rootInstanceId, game)