From 6e40993199b9f7891bef11a095f3acb5086c8f2b Mon Sep 17 00:00:00 2001 From: boatbomber Date: Thu, 13 Jul 2023 20:09:19 -0700 Subject: [PATCH] Rework patch visualizer with many fixes and improvements (#726) - Reconciler now has precommit and postcommit hooks for patch applying - This is used to compute a patch tree snapshot precommit and update the tree metadata postcommit - PatchVisualizer can now display Removes that happened during sync - It was previously missing because the removed objects no longer existed so it couldn't get any info on them (This is resolved because the info is gotten in precommit, before the instance was removed) - PatchVisualizer now shows Old and New values instead of just Incoming during sync - (Still displays Current and Incoming during confirmation) - This is much more useful, since you now see what the changes were and not just which things were changed - PatchVisualizer displays clarifying message when initial sync has no changes instead of just showing a blank box - Objects in the tree UI no longer get stuck expanded when the next patch has the same instance but different info on it - Objects in the tree UI correctly become selectable after their instance is added and unclickable when removed during sync --- CHANGELOG.md | 2 + .../Components/PatchVisualizer/ChangeList.lua | 11 +- .../Components/PatchVisualizer/DomLabel.lua | 20 +- .../App/Components/PatchVisualizer/init.lua | 464 +++--------------- plugin/src/App/StatusPages/Confirming.lua | 2 +- plugin/src/App/StatusPages/Connected.lua | 8 +- plugin/src/App/init.lua | 28 +- plugin/src/PatchTree.lua | 434 ++++++++++++++++ plugin/src/Reconciler/init.lua | 57 ++- 9 files changed, 612 insertions(+), 414 deletions(-) create mode 100644 plugin/src/PatchTree.lua diff --git a/CHANGELOG.md b/CHANGELOG.md index e36fb581d..130175e7e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ * Add buttons for navigation on the Connected page ([#722]) * Improve tooltip behavior ([#723]) * Better settings controls ([#725]) +* Rework patch visualizer with many fixes and improvements ([#726]) [#668]: https://github.com/rojo-rbx/rojo/pull/668 [#674]: https://github.com/rojo-rbx/rojo/pull/674 @@ -30,6 +31,7 @@ [#722]: https://github.com/rojo-rbx/rojo/pull/722 [#723]: https://github.com/rojo-rbx/rojo/pull/723 [#725]: https://github.com/rojo-rbx/rojo/pull/725 +[#726]: https://github.com/rojo-rbx/rojo/pull/726 ## [7.3.0] - April 22, 2023 * Added `$attributes` to project format. ([#574]) diff --git a/plugin/src/App/Components/PatchVisualizer/ChangeList.lua b/plugin/src/App/Components/PatchVisualizer/ChangeList.lua index b1fa54da4..ed6da37a6 100644 --- a/plugin/src/App/Components/PatchVisualizer/ChangeList.lua +++ b/plugin/src/App/Components/PatchVisualizer/ChangeList.lua @@ -8,6 +8,8 @@ local Theme = require(Plugin.App.Theme) local ScrollingFrame = require(Plugin.App.Components.ScrollingFrame) local DisplayValue = require(script.Parent.DisplayValue) +local EMPTY_TABLE = {} + local e = Roact.createElement local ChangeList = Roact.Component:extend("ChangeList") @@ -20,7 +22,6 @@ function ChangeList:render() return Theme.with(function(theme) local props = self.props local changes = props.changes - local columnVisibility = props.columnVisibility -- Color alternating rows for readability local rowTransparency = props.transparency:map(function(t) @@ -47,7 +48,6 @@ function ChangeList:render() VerticalAlignment = Enum.VerticalAlignment.Center, }), A = e("TextLabel", { - Visible = columnVisibility[1], Text = tostring(changes[1][1]), BackgroundTransparency = 1, Font = Enum.Font.GothamBold, @@ -60,7 +60,6 @@ function ChangeList:render() LayoutOrder = 1, }), B = e("TextLabel", { - Visible = columnVisibility[2], Text = tostring(changes[1][2]), BackgroundTransparency = 1, Font = Enum.Font.GothamBold, @@ -73,7 +72,6 @@ function ChangeList:render() LayoutOrder = 2, }), C = e("TextLabel", { - Visible = columnVisibility[3], Text = tostring(changes[1][3]), BackgroundTransparency = 1, Font = Enum.Font.GothamBold, @@ -92,7 +90,7 @@ function ChangeList:render() continue -- Skip headers, already handled above end - local metadata = values[4] + local metadata = values[4] or EMPTY_TABLE local isWarning = metadata.isWarning rows[row] = e("Frame", { @@ -110,7 +108,6 @@ function ChangeList:render() VerticalAlignment = Enum.VerticalAlignment.Center, }), A = e("TextLabel", { - Visible = columnVisibility[1], Text = (if isWarning then "⚠ " else "") .. tostring(values[1]), BackgroundTransparency = 1, Font = Enum.Font.GothamMedium, @@ -125,7 +122,6 @@ function ChangeList:render() B = e( "Frame", { - Visible = columnVisibility[2], BackgroundTransparency = 1, Size = UDim2.new(0.35, 0, 1, 0), LayoutOrder = 2, @@ -139,7 +135,6 @@ function ChangeList:render() C = e( "Frame", { - Visible = columnVisibility[3], BackgroundTransparency = 1, Size = UDim2.new(0.35, 0, 1, 0), LayoutOrder = 3, diff --git a/plugin/src/App/Components/PatchVisualizer/DomLabel.lua b/plugin/src/App/Components/PatchVisualizer/DomLabel.lua index 98a4c0ff3..facf70cd9 100644 --- a/plugin/src/App/Components/PatchVisualizer/DomLabel.lua +++ b/plugin/src/App/Components/PatchVisualizer/DomLabel.lua @@ -34,7 +34,6 @@ function Expansion:render() ChangeList = e(ChangeList, { changes = props.changeList, transparency = props.transparency, - columnVisibility = props.columnVisibility, }), }) end @@ -71,6 +70,22 @@ function DomLabel:init() end) end +function DomLabel:didUpdate(prevProps) + if + prevProps.instance ~= self.props.instance + or prevProps.patchType ~= self.props.patchType + or prevProps.name ~= self.props.name + or prevProps.changeList ~= self.props.changeList + then + -- Close the expansion when the domlabel is changed to a different thing + self.expanded = false + self.motor:setGoal(Flipper.Spring.new(30, { + frequency = 5, + dampingRatio = 1, + })) + end +end + function DomLabel:render() local props = self.props @@ -130,7 +145,7 @@ function DomLabel:render() if props.changeList then self.expanded = not self.expanded local goalHeight = 30 - + (if self.expanded then math.clamp(#self.props.changeList * 30, 30, 30 * 6) else 0) + + (if self.expanded then math.clamp(#props.changeList * 30, 30, 30 * 6) else 0) self.motor:setGoal(Flipper.Spring.new(goalHeight, { frequency = 5, dampingRatio = 1, @@ -155,7 +170,6 @@ function DomLabel:render() indent = indent, transparency = props.transparency, changeList = props.changeList, - columnVisibility = props.columnVisibility, }) else nil, DiffIcon = if props.patchType diff --git a/plugin/src/App/Components/PatchVisualizer/init.lua b/plugin/src/App/Components/PatchVisualizer/init.lua index be1fc8c65..73af45029 100644 --- a/plugin/src/App/Components/PatchVisualizer/init.lua +++ b/plugin/src/App/Components/PatchVisualizer/init.lua @@ -1,154 +1,18 @@ -local HttpService = game:GetService("HttpService") - local Rojo = script:FindFirstAncestor("Rojo") local Plugin = Rojo.Plugin local Packages = Rojo.Packages local Roact = require(Packages.Roact) -local Log = require(Packages.Log) -local Types = require(Plugin.Types) +local PatchTree = require(Plugin.PatchTree) local PatchSet = require(Plugin.PatchSet) -local decodeValue = require(Plugin.Reconciler.decodeValue) -local getProperty = require(Plugin.Reconciler.getProperty) +local Theme = require(Plugin.App.Theme) local BorderedContainer = require(Plugin.App.Components.BorderedContainer) local VirtualScroller = require(Plugin.App.Components.VirtualScroller) local e = Roact.createElement -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 - -- table being iterated. - - local key = nil - if state == nil then - -- First iteration, generate the index - local orderedIndex, i = table.create(5), 0 - for k in t do - i += 1 - orderedIndex[i] = k - end - table.sort(orderedIndex, function(a, b) - local nodeA, nodeB = t[a], t[b] - return (nodeA.name or "") < (nodeB.name or "") - end) - - t.__orderedIndex = orderedIndex - key = orderedIndex[1] - else - -- Fetch the next value - for i, orderedState in t.__orderedIndex do - if orderedState == state then - key = t.__orderedIndex[i + 1] - break - end - end - end - - if key then - return key, t[key] - end - - -- No more value to return, cleanup - t.__orderedIndex = nil - return -end - -local function alphabeticalPairs(t) - -- Equivalent of the pairs() iterator, but sorted - return alphabeticalNext, t, nil -end - -local function Tree() - local tree = { - idToNode = {}, - ROOT = { - className = "DataModel", - name = "ROOT", - children = {}, - }, - } - -- Add ROOT to idToNode or it won't be found by getNode since that searches *within* ROOT - tree.idToNode["ROOT"] = tree.ROOT - - function tree:getNode(id, target) - if self.idToNode[id] then - return self.idToNode[id] - end - - for nodeId, node in target or tree.ROOT.children do - if nodeId == id then - self.idToNode[id] = node - return node - end - local descendant = self:getNode(id, node.children) - if descendant then - return descendant - end - end - - return nil - end - - function tree:addNode(parent, props) - parent = parent or "ROOT" - - local node = self:getNode(props.id) - if node then - for k, v in props do - node[k] = v - end - return node - end - - node = table.clone(props) - node.children = {} - - local parentNode = self:getNode(parent) - if not parentNode then - Log.warn("Failed to create node since parent doesnt exist: {}, {}", parent, props) - return - end - - parentNode.children[node.id] = node - self.idToNode[node.id] = node - - return node - end - - function tree:buildAncestryNodes(ancestry, patch, instanceMap) - -- Build nodes for ancestry by going up the tree - local previousId = "ROOT" - for _, ancestorId in ancestry do - local value = instanceMap.fromIds[ancestorId] or patch.added[ancestorId] - if not value then - Log.warn("Failed to find ancestor object for " .. ancestorId) - continue - end - self:addNode(previousId, { - id = ancestorId, - className = value.ClassName, - name = value.Name, - instance = if typeof(value) == "Instance" then value else nil, - }) - previousId = ancestorId - end - end - - return tree -end - -local function findUnappliedPropsForId(unappliedPatch, id) - for _, change in unappliedPatch.updated do - if change.id == id then - return change.changedProperties or {} - end - end - return {} -end - local DomLabel = require(script.DomLabel) local PatchVisualizer = Roact.Component:extend("PatchVisualizer") @@ -164,274 +28,90 @@ function PatchVisualizer:willUnmount() end function PatchVisualizer:shouldUpdate(nextProps) - local currentPatch, nextPatch = self.props.patch, nextProps.patch - - return not PatchSet.isEqual(currentPatch, nextPatch) -end - -function PatchVisualizer:buildTree(patch, unappliedPatch, instanceMap) - local tree = Tree() - - for _, change in patch.updated do - local instance = instanceMap.fromIds[change.id] - if not instance then - continue - end - - -- Gather ancestors from existing DOM - local ancestry = {} - local parentObject = instance.Parent - local parentId = instanceMap.fromInstances[parentObject] - while parentObject do - table.insert(ancestry, 1, parentId) - parentObject = parentObject.Parent - parentId = instanceMap.fromInstances[parentObject] - end - - tree:buildAncestryNodes(ancestry, patch, instanceMap) - - -- Gather detail text - local changeList, hint = nil, nil - if next(change.changedProperties) or change.changedName then - local unappliedChanges = findUnappliedPropsForId(unappliedPatch, change.id) - - changeList = {} - - local hintBuffer, i = {}, 0 - local function addProp(prop: string, current: any?, incoming: any?, metadata: any?) - i += 1 - hintBuffer[i] = prop - changeList[i] = { prop, current, incoming, metadata } - end - - -- Gather the changes - - if change.changedName then - addProp("Name", instance.Name, change.changedName) - end - - for prop, incoming in change.changedProperties do - local incomingSuccess, incomingValue = decodeValue(incoming, instanceMap) - local currentSuccess, currentValue = getProperty(instance, prop) - - addProp( - prop, - if currentSuccess then currentValue else "[Error]", - if incomingSuccess then incomingValue else next(incoming), - { - isWarning = unappliedChanges[prop] ~= nil - } - ) - 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, ", ") - - -- Sort changes and add header - table.sort(changeList, function(a, b) - return a[1] < b[1] - end) - table.insert(changeList, 1, { "Property", "Current", "Incoming" }) - end - - -- Add this node to tree - tree:addNode(instanceMap.fromInstances[instance.Parent], { - id = change.id, - patchType = "Edit", - className = instance.ClassName, - name = instance.Name, - instance = instance, - hint = hint, - changeList = changeList, - }) - end - - for _, idOrInstance in patch.removed do - 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 - -- and we therefore cannot get the tree for it anymore - continue - end - - -- Gather ancestors from existing DOM - -- (note that they may have no ID if they're being removed as unknown) - local ancestry = {} - local parentObject = instance.Parent - local parentId = instanceMap.fromInstances[parentObject] or HttpService:GenerateGUID(false) - while parentObject do - instanceMap:insert(parentId, parentObject) - table.insert(ancestry, 1, parentId) - parentObject = parentObject.Parent - parentId = instanceMap.fromInstances[parentObject] or HttpService:GenerateGUID(false) - end - - tree:buildAncestryNodes(ancestry, patch, instanceMap) - - -- Add this node to tree - local nodeId = instanceMap.fromInstances[instance] or HttpService:GenerateGUID(false) - instanceMap:insert(nodeId, instance) - tree:addNode(instanceMap.fromInstances[instance.Parent], { - id = nodeId, - patchType = "Remove", - className = instance.ClassName, - name = instance.Name, - instance = instance, - }) + if self.props.patchTree ~= nextProps.patchTree then + return true end - for id, change in patch.added do - -- Gather ancestors from existing DOM or future additions - local ancestry = {} - local parentId = change.Parent - local parentData = patch.added[parentId] - local parentObject = instanceMap.fromIds[parentId] - while parentId do - table.insert(ancestry, 1, parentId) - parentId = nil - - if parentData then - parentId = parentData.Parent - parentData = patch.added[parentId] - parentObject = instanceMap.fromIds[parentId] - elseif parentObject then - parentObject = parentObject.Parent - parentId = instanceMap.fromInstances[parentObject] - parentData = patch.added[parentId] - end - end - - tree:buildAncestryNodes(ancestry, patch, instanceMap) - - -- Gather detail text - local changeList, hint = nil, nil - if next(change.Properties) then - local unappliedChanges = findUnappliedPropsForId(unappliedPatch, change.Id) - - changeList = {} - - local hintBuffer, i = {}, 0 - for prop, incoming in change.Properties do - i += 1 - hintBuffer[i] = prop - - local success, incomingValue = decodeValue(incoming, instanceMap) - if success then - table.insert(changeList, { prop, "N/A", incomingValue, { - isWarning = unappliedChanges[prop] ~= nil - } }) - else - table.insert(changeList, { prop, "N/A", next(incoming), { - isWarning = unappliedChanges[prop] ~= nil - } }) - 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", - } - end - hint = table.concat(hintBuffer, ", ") - - -- Sort changes and add header - table.sort(changeList, function(a, b) - return a[1] < b[1] - end) - table.insert(changeList, 1, { "Property", "Current", "Incoming" }) - end - - -- Add this node to tree - tree:addNode(change.Parent, { - id = change.Id, - patchType = "Add", - className = change.ClassName, - name = change.Name, - hint = hint, - changeList = changeList, - instance = instanceMap.fromIds[id], - }) + local currentPatch, nextPatch = self.props.patch, nextProps.patch + if currentPatch ~= nil or nextPatch ~= nil then + return not PatchSet.isEqual(currentPatch, nextPatch) end - return tree + return false end function PatchVisualizer:render() - local patch = self.props.patch or PatchSet.newEmpty() - local unappliedPatch = self.props.unappliedPatch or PatchSet.newEmpty() - local instanceMap = self.props.instanceMap - - local tree = self:buildTree(patch, unappliedPatch, instanceMap) + local patchTree = self.props.patchTree + if patchTree == nil and self.props.patch ~= nil then + patchTree = PatchTree.build(self.props.patch, self.props.instanceMap, self.props.changeListHeaders or { "Property", "Current", "Incoming" }) + if self.props.unappliedPatch then + patchTree = PatchTree.updateMetadata(patchTree, self.props.patch, self.props.instanceMap, self.props.unappliedPatch) + end + end -- Recusively draw tree local scrollElements, elementHeights = {}, {} - local function drawNode(node, depth) - local elementHeight, setElementHeight = Roact.createBinding(30) - table.insert(elementHeights, elementHeight) - table.insert( - scrollElements, - e(DomLabel, { - columnVisibility = self.props.columnVisibility, - updateEvent = self.updateEvent, - elementHeight = elementHeight, - setElementHeight = setElementHeight, - patchType = node.patchType, - className = node.className, - isWarning = next(findUnappliedPropsForId(unappliedPatch, node.id)) ~= nil, - instance = node.instance, - name = node.name, - hint = node.hint, - changeList = node.changeList, - depth = depth, - transparency = self.props.transparency, - }) - ) - for _, childNode in alphabeticalPairs(node.children) do - drawNode(childNode, depth + 1) - end - end - for _, node in alphabeticalPairs(tree.ROOT.children) do - drawNode(node, 0) + if patchTree then + local function drawNode(node, depth) + local elementHeight, setElementHeight = Roact.createBinding(30) + table.insert(elementHeights, elementHeight) + table.insert( + scrollElements, + e(DomLabel, { + updateEvent = self.updateEvent, + elementHeight = elementHeight, + setElementHeight = setElementHeight, + patchType = node.patchType, + className = node.className, + isWarning = node.isWarning, + instance = node.instance, + name = node.name, + hint = node.hint, + changeList = node.changeList, + depth = depth, + transparency = self.props.transparency, + }) + ) + end + + patchTree:forEach(function(node, depth) + drawNode(node, depth) + end) end - return e(BorderedContainer, { - transparency = self.props.transparency, - size = self.props.size, - position = self.props.position, - layoutOrder = self.props.layoutOrder, - }, { - VirtualScroller = e(VirtualScroller, { - size = UDim2.new(1, 0, 1, 0), + return Theme.with(function(theme) + return e(BorderedContainer, { transparency = self.props.transparency, - count = #scrollElements, - updateEvent = self.updateEvent.Event, - render = function(i) - return scrollElements[i] - end, - getHeightBinding = function(i) - return elementHeights[i] - end, - }), - }) + size = self.props.size, + position = self.props.position, + layoutOrder = self.props.layoutOrder, + }, { + CleanMerge = e("TextLabel", { + Visible = #scrollElements == 0, + Text = "No changes to sync, project is up to date.", + Font = Enum.Font.GothamMedium, + TextSize = 15, + TextColor3 = theme.Settings.Setting.DescriptionColor, + TextWrapped = true, + Size = UDim2.new(1, 0, 1, 0), + BackgroundTransparency = 1, + }), + + VirtualScroller = e(VirtualScroller, { + size = UDim2.new(1, 0, 1, 0), + transparency = self.props.transparency, + count = #scrollElements, + updateEvent = self.updateEvent.Event, + render = function(i) + return scrollElements[i] + end, + getHeightBinding = function(i) + return elementHeights[i] + end, + }), + }) + end) end return PatchVisualizer diff --git a/plugin/src/App/StatusPages/Confirming.lua b/plugin/src/App/StatusPages/Confirming.lua index 3359eaf61..a55923470 100644 --- a/plugin/src/App/StatusPages/Confirming.lua +++ b/plugin/src/App/StatusPages/Confirming.lua @@ -52,7 +52,7 @@ function ConfirmingPage:render() transparency = self.props.transparency, layoutOrder = 3, - columnVisibility = {true, true, true}, + changeListHeaders = { "Property", "Current", "Incoming" }, patch = self.props.confirmData.patch, instanceMap = self.props.confirmData.instanceMap, }), diff --git a/plugin/src/App/StatusPages/Connected.lua b/plugin/src/App/StatusPages/Connected.lua index 689b90220..00f6822bb 100644 --- a/plugin/src/App/StatusPages/Connected.lua +++ b/plugin/src/App/StatusPages/Connected.lua @@ -83,10 +83,7 @@ function ChangesDrawer:render() transparency = self.props.transparency, layoutOrder = 3, - columnVisibility = { true, false, true }, - patch = self.props.patch, - unappliedPatch = self.props.unappliedPatch, - instanceMap = self.serveSession.__instanceMap, + patchTree = self.props.patchTree, }), }) end) @@ -365,8 +362,7 @@ function ConnectedPage:render() ChangesDrawer = e(ChangesDrawer, { rendered = self.state.renderChanges, transparency = self.props.transparency, - patch = self.props.patchData.patch, - unappliedPatch = self.props.patchData.unapplied, + patchTree = self.props.patchTree, serveSession = self.props.serveSession, height = self.changeDrawerHeight, layoutOrder = 5, diff --git a/plugin/src/App/init.lua b/plugin/src/App/init.lua index fd534d0c8..23d0a5069 100644 --- a/plugin/src/App/init.lua +++ b/plugin/src/App/init.lua @@ -19,6 +19,7 @@ local Dictionary = require(Plugin.Dictionary) local ServeSession = require(Plugin.ServeSession) local ApiContext = require(Plugin.ApiContext) local PatchSet = require(Plugin.PatchSet) +local PatchTree = require(Plugin.PatchTree) local preloadAssets = require(Plugin.preloadAssets) local soundPlayer = require(Plugin.soundPlayer) local ignorePlaceIds = require(Plugin.ignorePlaceIds) @@ -358,6 +359,21 @@ function App:startSession() twoWaySync = sessionOptions.twoWaySync, }) + self.cleanupPrecommit = serveSession.__reconciler:hookPrecommit(function(patch, instanceMap) + -- Build new tree for patch + self:setState({ + patchTree = PatchTree.build(patch, instanceMap, { "Property", "Old", "New" }), + }) + end) + self.cleanupPostcommit = serveSession.__reconciler:hookPostcommit(function(patch, instanceMap, unappliedPatch) + -- Update tree with unapplied metadata + self:setState(function(prevState) + return { + patchTree = PatchTree.updateMetadata(prevState.patchTree, patch, instanceMap, unappliedPatch), + } + end) + end) + serveSession:onPatchApplied(function(patch, unapplied) local now = os.time() local old = self.state.patchData @@ -503,6 +519,13 @@ function App:endSession() appStatus = AppStatus.NotConnected, }) + if self.cleanupPrecommit ~= nil then + self.cleanupPrecommit() + end + if self.cleanupPostcommit ~= nil then + self.cleanupPostcommit() + end + Log.trace("Session terminated by user") end @@ -533,8 +556,8 @@ function App:render() initDockState = Enum.InitialDockState.Right, initEnabled = false, overridePreviousState = false, - floatingSize = Vector2.new(300, 200), - minimumSize = Vector2.new(300, 120), + floatingSize = Vector2.new(320, 210), + minimumSize = Vector2.new(300, 210), zIndexBehavior = Enum.ZIndexBehavior.Sibling, @@ -590,6 +613,7 @@ function App:render() Connected = createPageElement(AppStatus.Connected, { projectName = self.state.projectName, address = self.state.address, + patchTree = self.state.patchTree, patchData = self.state.patchData, serveSession = self.serveSession, diff --git a/plugin/src/PatchTree.lua b/plugin/src/PatchTree.lua new file mode 100644 index 000000000..b241472d0 --- /dev/null +++ b/plugin/src/PatchTree.lua @@ -0,0 +1,434 @@ +--[[ + Methods to turn PatchSets into trees matching the DataModel containing + the changes and metadata for use in the PatchVisualizer component. +]] + +local HttpService = game:GetService("HttpService") + +local Rojo = script:FindFirstAncestor("Rojo") +local Plugin = Rojo.Plugin +local Packages = Rojo.Packages + +local Log = require(Packages.Log) + +local Types = require(Plugin.Types) +local decodeValue = require(Plugin.Reconciler.decodeValue) +local getProperty = require(Plugin.Reconciler.getProperty) + +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 + -- table being iterated. + + local key = nil + if state == nil then + -- First iteration, generate the index + local orderedIndex, i = table.create(5), 0 + for k in t do + i += 1 + orderedIndex[i] = k + end + table.sort(orderedIndex, function(a, b) + local nodeA, nodeB = t[a], t[b] + return (nodeA.name or "") < (nodeB.name or "") + end) + + t.__orderedIndex = orderedIndex + key = orderedIndex[1] + else + -- Fetch the next value + for i, orderedState in t.__orderedIndex do + if orderedState == state then + key = t.__orderedIndex[i + 1] + break + end + end + end + + if key then + return key, t[key] + end + + -- No more value to return, cleanup + t.__orderedIndex = nil + + return +end + +local function alphabeticalPairs(t) + -- Equivalent of the pairs() iterator, but sorted + return alphabeticalNext, t, nil +end + +local Tree = {} +Tree.__index = Tree + +function Tree.new() + local tree = { + idToNode = {}, + ROOT = { + className = "DataModel", + name = "ROOT", + children = {}, + }, + } + -- Add ROOT to idToNode or it won't be found by getNode since that searches *within* ROOT + tree.idToNode["ROOT"] = tree.ROOT + + return setmetatable(tree, Tree) +end + +-- Iterates over all sub-nodes, depth first +-- node is where to start from, defaults to root +-- depth is used for recursion but can be used to set the starting depth +function Tree:forEach(callback, node, depth) + depth = depth or 1 + for _, child in alphabeticalPairs(if node then node.children else self.ROOT.children) do + callback(child, depth) + if type(child.children) == "table" then + self:forEach(callback, child, depth + 1) + end + end +end + +-- Finds a node by id, depth first +-- searchNode is the node to start the search within, defaults to root +function Tree:getNode(id, searchNode) + if self.idToNode[id] then + return self.idToNode[id] + end + + local searchChildren = (searchNode or self.ROOT).children + for nodeId, node in searchChildren do + if nodeId == id then + self.idToNode[id] = node + return node + end + local descendant = self:getNode(id, node) + if descendant then + return descendant + end + end + + return nil +end + +-- Adds a node to the tree as a child of the node with id == parent +-- If parent is nil, it defaults to root +-- props must contain id, and cannot contain children or parentId +-- other than those three, it can hold anything +function Tree:addNode(parent, props) + assert(props.id, "props must contain id") + + parent = parent or "ROOT" + + local node = self:getNode(props.id) + if node then + -- Update existing node + for k, v in props do + node[k] = v + end + return node + end + + node = table.clone(props) + node.children = {} + node.parentId = parent + + local parentNode = self:getNode(parent) + if not parentNode then + Log.warn("Failed to create node since parent doesnt exist: {}, {}", parent, props) + return + end + + parentNode.children[node.id] = node + self.idToNode[node.id] = node + + 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(ancestryIds: { string }, patch, instanceMap) + -- Build nodes for ancestry by going up the tree + local previousId = "ROOT" + for _, ancestorId in ancestryIds do + local value = instanceMap.fromIds[ancestorId] or patch.added[ancestorId] + if not value then + Log.warn("Failed to find ancestor object for " .. ancestorId) + continue + end + self:addNode(previousId, { + id = ancestorId, + className = value.ClassName, + name = value.Name, + instance = if typeof(value) == "Instance" then value else nil, + }) + previousId = ancestorId + end +end + +local PatchTree = {} + +-- Builds a new tree from a patch and instanceMap +-- uses changeListHeaders in node.changeList +function PatchTree.build(patch, instanceMap, changeListHeaders) + local tree = Tree.new() + + for _, change in patch.updated do + local instance = instanceMap.fromIds[change.id] + if not instance then + continue + end + + -- Gather ancestors from existing DOM + local ancestryIds = {} + local parentObject = instance.Parent + local parentId = instanceMap.fromInstances[parentObject] + while parentObject do + table.insert(ancestryIds, 1, parentId) + parentObject = parentObject.Parent + parentId = instanceMap.fromInstances[parentObject] + end + + tree:buildAncestryNodes(ancestryIds, patch, instanceMap) + + -- Gather detail text + local changeList, hint = nil, nil + if next(change.changedProperties) or change.changedName then + changeList = {} + + local hintBuffer, i = {}, 0 + local function addProp(prop: string, current: any?, incoming: any?, metadata: any?) + i += 1 + hintBuffer[i] = prop + changeList[i] = { prop, current, incoming, metadata } + end + + -- Gather the changes + + if change.changedName then + addProp("Name", instance.Name, change.changedName) + end + + for prop, incoming in change.changedProperties do + local incomingSuccess, incomingValue = decodeValue(incoming, instanceMap) + local currentSuccess, currentValue = getProperty(instance, prop) + + addProp( + prop, + if currentSuccess then currentValue else "[Error]", + if incomingSuccess then incomingValue else next(incoming) + ) + 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, ", ") + + -- Sort changes and add header + table.sort(changeList, function(a, b) + return a[1] < b[1] + end) + table.insert(changeList, 1, changeListHeaders) + end + + -- Add this node to tree + tree:addNode(instanceMap.fromInstances[instance.Parent], { + id = change.id, + patchType = "Edit", + className = instance.ClassName, + name = instance.Name, + instance = instance, + hint = hint, + changeList = changeList, + }) + end + + for _, idOrInstance in patch.removed do + 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 + -- and we therefore cannot get the tree for it anymore + continue + end + + -- Gather ancestors from existing DOM + -- (note that they may have no ID if they're being removed as unknown) + local ancestryIds = {} + local parentObject = instance.Parent + local parentId = instanceMap.fromInstances[parentObject] or HttpService:GenerateGUID(false) + while parentObject do + instanceMap:insert(parentId, parentObject) -- This ensures we can find the parent later + table.insert(ancestryIds, 1, parentId) + parentObject = parentObject.Parent + parentId = instanceMap.fromInstances[parentObject] or HttpService:GenerateGUID(false) + end + + tree:buildAncestryNodes(ancestryIds, patch, instanceMap) + + -- Add this node to tree + local nodeId = instanceMap.fromInstances[instance] or HttpService:GenerateGUID(false) + instanceMap:insert(nodeId, instance) + tree:addNode(instanceMap.fromInstances[instance.Parent], { + id = nodeId, + patchType = "Remove", + className = instance.ClassName, + name = instance.Name, + instance = instance, + }) + end + + for id, change in patch.added do + -- Gather ancestors from existing DOM or future additions + local ancestryIds = {} + local parentId = change.Parent + local parentData = patch.added[parentId] + local parentObject = instanceMap.fromIds[parentId] + while parentId do + table.insert(ancestryIds, 1, parentId) + parentId = nil + + if parentData then + -- object is parented to an instance that does not exist yet + parentId = parentData.Parent + parentData = patch.added[parentId] + parentObject = instanceMap.fromIds[parentId] + elseif parentObject then + -- object is parented to an instance that exists + parentObject = parentObject.Parent + parentId = instanceMap.fromInstances[parentObject] + parentData = patch.added[parentId] + end + end + + tree:buildAncestryNodes(ancestryIds, patch, instanceMap) + + -- Gather detail text + local changeList, hint = nil, nil + if next(change.Properties) then + changeList = {} + + local hintBuffer, i = {}, 0 + for prop, incoming in change.Properties do + i += 1 + hintBuffer[i] = prop + + local success, incomingValue = decodeValue(incoming, instanceMap) + if success then + table.insert(changeList, { prop, "N/A", incomingValue }) + else + table.insert(changeList, { prop, "N/A", next(incoming) }) + 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", + } + end + hint = table.concat(hintBuffer, ", ") + + -- Sort changes and add header + table.sort(changeList, function(a, b) + return a[1] < b[1] + end) + table.insert(changeList, 1, changeListHeaders) + end + + -- Add this node to tree + tree:addNode(change.Parent, { + id = change.Id, + patchType = "Add", + className = change.ClassName, + name = change.Name, + hint = hint, + changeList = changeList, + instance = instanceMap.fromIds[id], + }) + end + + 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) + if tree then + tree = PatchTree.clone(tree) + else + tree = PatchTree.build(patch, instanceMap) + end + + -- Update isWarning metadata + for _, failedChange in unappliedPatch.updated do + local node = tree:getNode(failedChange.id) + if node then + node.isWarning = true + Log.trace("Marked node as warning: {} {}", node.id, node.name) + + if node.changeList then + for _, change in node.changeList do + if failedChange.changedProperties[change[1]] then + Log.trace(" Marked property as warning: {}", change[1]) + if change[4] == nil then + change[4] = {} + end + change[4].isWarning = true + end + end + end + end + end + + -- Update if instances exist + tree:forEach(function(node) + if node.instance then + if node.instance.Parent == nil and node.instance ~= game then + -- This instance has been removed + Log.trace("Removed instance from node: {} {}", node.id, node.name) + node.instance = nil + end + else + -- This instance may have been added + node.instance = instanceMap.fromIds[node.id] + if node.instance then + Log.trace("Added instance to node: {} {}", node.id, node.name) + end + end + end) + + return tree +end + +return PatchTree diff --git a/plugin/src/Reconciler/init.lua b/plugin/src/Reconciler/init.lua index 1db295a40..129457827 100644 --- a/plugin/src/Reconciler/init.lua +++ b/plugin/src/Reconciler/init.lua @@ -3,6 +3,9 @@ and mutating the Roblox DOM. ]] +local Packages = script.Parent.Parent.Packages +local Log = require(Packages.Log) + local applyPatch = require(script.applyPatch) local hydrate = require(script.hydrate) local diff = require(script.diff) @@ -14,13 +17,63 @@ function Reconciler.new(instanceMap) local self = { -- Tracks all of the instances known by the reconciler by ID. __instanceMap = instanceMap, + __precommitCallbacks = {}, + __postcommitCallbacks = {}, } return setmetatable(self, Reconciler) end +function Reconciler:hookPrecommit(callback: (patch: any, instanceMap: any) -> ()): () -> () + table.insert(self.__precommitCallbacks, callback) + Log.trace("Added precommit callback: {}", callback) + + return function() + -- Remove the callback from the list + for i, cb in self.__precommitCallbacks do + if cb == callback then + table.remove(self.__precommitCallbacks, i) + Log.trace("Removed precommit callback: {}", callback) + break + end + end + end +end + +function Reconciler:hookPostcommit(callback: (patch: any, instanceMap: any, unappliedPatch: any) -> ()): () -> () + table.insert(self.__postcommitCallbacks, callback) + Log.trace("Added postcommit callback: {}", callback) + + return function() + -- Remove the callback from the list + for i, cb in self.__postcommitCallbacks do + if cb == callback then + table.remove(self.__postcommitCallbacks, i) + Log.trace("Removed postcommit callback: {}", callback) + break + end + end + end +end + function Reconciler:applyPatch(patch) - return applyPatch(self.__instanceMap, patch) + 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 + + local unappliedPatch = applyPatch(self.__instanceMap, patch) + + 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 + end + + return unappliedPatch end function Reconciler:hydrate(virtualInstances, rootId, rootInstance) @@ -31,4 +84,4 @@ function Reconciler:diff(virtualInstances, rootId) return diff(self.__instanceMap, virtualInstances, rootId) end -return Reconciler \ No newline at end of file +return Reconciler