From ce530e795a51026936c3dee46eb34cad00c0c4ff Mon Sep 17 00:00:00 2001 From: boatbomber Date: Wed, 5 Jul 2023 14:09:11 -0700 Subject: [PATCH 01/18] Fix Rojo breaking when users undo/redo in Studio (#708) --- CHANGELOG.md | 4 +- plugin/src/App/init.lua | 105 ++++++++++++++++++++++++--- plugin/src/Reconciler/applyPatch.lua | 6 ++ 3 files changed, 104 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ec66d3c6..373eef77 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ * Skip confirming patches that contain only a datamodel name change. ([#688]) * Added sync reminder notification. ([#689]) * Added protection against syncing a model to a place. ([#691]) +* Fix Rojo breaking when users undo/redo in Studio ([#708]) [#668]: https://github.com/rojo-rbx/rojo/pull/668 [#674]: https://github.com/rojo-rbx/rojo/pull/674 @@ -14,6 +15,7 @@ [#688]: https://github.com/rojo-rbx/rojo/pull/688 [#689]: https://github.com/rojo-rbx/rojo/pull/689 [#691]: https://github.com/rojo-rbx/rojo/pull/691 +[#708]: https://github.com/rojo-rbx/rojo/pull/708 ## [7.3.0] - April 22, 2023 * Added `$attributes` to project format. ([#574]) @@ -579,4 +581,4 @@ This is a general maintenance release for the Rojo 0.5.x release series. * More robust syncing with a new reconciler ## [0.1.0](https://github.com/rojo-rbx/rojo/releases/tag/v0.1.0) (November 29, 2017) -* Initial release, functionally very similar to [rbxfs](https://github.com/LPGhatguy/rbxfs) \ No newline at end of file +* Initial release, functionally very similar to [rbxfs](https://github.com/LPGhatguy/rbxfs) diff --git a/plugin/src/App/init.lua b/plugin/src/App/init.lua index 9fe081d7..c7be7671 100644 --- a/plugin/src/App/init.lua +++ b/plugin/src/App/init.lua @@ -1,3 +1,4 @@ +local ChangeHistoryService = game:GetService("ChangeHistoryService") local Players = game:GetService("Players") local ServerStorage = game:GetService("ServerStorage") local RunService = game:GetService("RunService") @@ -57,6 +58,64 @@ function App:init() self.confirmationEvent = self.confirmationBindable.Event self.notifId = 0 + self.waypointConnection = ChangeHistoryService.OnUndo:Connect(function(action: string) + if not string.find(action, "^Rojo: Patch") then + return + end + + local undoConnection, redoConnection = nil, nil + local function cleanup() + undoConnection:Disconnect() + redoConnection:Disconnect() + end + + Log.warn( + string.format( + "You've undone '%s'.\nIf this was not intended, please Redo in the topbar or with Ctrl/⌘+Y.", + action + ) + ) + local dismissNotif = self:addNotification( + string.format("You've undone '%s'.\nIf this was not intended, please restore.", action), + 10, + { + Restore = { + text = "Restore", + style = "Solid", + layoutOrder = 1, + onClick = function(notification) + cleanup() + notification:dismiss() + ChangeHistoryService:Redo() + end, + }, + Dismiss = { + text = "Dismiss", + style = "Bordered", + layoutOrder = 2, + onClick = function(notification) + cleanup() + notification:dismiss() + end, + }, + } + ) + + undoConnection = ChangeHistoryService.OnUndo:Once(function() + -- Our notif is now out of date- redoing will not restore the patch + -- since we've undone even further. Dismiss the notif. + cleanup() + dismissNotif() + end) + redoConnection = ChangeHistoryService.OnRedo:Once(function(redoneAction: string) + if redoneAction == action then + -- The user has restored the patch, so we can dismiss the notif + cleanup() + dismissNotif() + end + end) + end) + self:setState({ appStatus = AppStatus.NotConnected, guiEnabled = false, @@ -83,7 +142,7 @@ function App:init() onClick = function(notification) notification:dismiss() self:startSession() - end + end, }, Dismiss = { text = "Dismiss", @@ -97,7 +156,16 @@ function App:init() end end -function App:addNotification(text: string, timeout: number?, actions: { [string]: {text: string, style: string, layoutOrder: number, onClick: (any) -> ()} }?) +function App:willUnmount() + self.waypointConnection:Disconnect() + self.confirmationBindable:Destroy() +end + +function App:addNotification( + text: string, + timeout: number?, + actions: { [string]: { text: string, style: string, layoutOrder: number, onClick: (any) -> () } }? +) if not Settings:get("showNotifications") then return end @@ -123,6 +191,10 @@ function App:addNotification(text: string, timeout: number?, actions: { [string] end function App:closeNotification(id: number) + if not self.state.notifications[id] then + return + end + local notifications = table.clone(self.state.notifications) notifications[id] = nil @@ -133,26 +205,38 @@ end function App:getPriorEndpoint() local priorEndpoints = Settings:get("priorEndpoints") - if not priorEndpoints then return end + if not priorEndpoints then + return + end local id = tostring(game.PlaceId) - if ignorePlaceIds[id] then return end + if ignorePlaceIds[id] then + return + end local place = priorEndpoints[id] - if not place then return end + if not place then + return + end return place.host, place.port end function App:getLastSyncTimestamp() local priorEndpoints = Settings:get("priorEndpoints") - if not priorEndpoints then return end + if not priorEndpoints then + return + end local id = tostring(game.PlaceId) - if ignorePlaceIds[id] then return end + if ignorePlaceIds[id] then + return + end local place = priorEndpoints[id] - if not place then return end + if not place then + return + end return place.timestamp end @@ -172,7 +256,9 @@ function App:setPriorEndpoint(host: string, port: string) end local id = tostring(game.PlaceId) - if ignorePlaceIds[id] then return end + if ignorePlaceIds[id] then + return + end priorEndpoints[id] = { host = if host ~= Config.defaultHost then host else nil, @@ -181,7 +267,6 @@ function App:setPriorEndpoint(host: string, port: string) } Log.trace("Saved last used endpoint for {}", game.PlaceId) - Settings:set("priorEndpoints", priorEndpoints) end diff --git a/plugin/src/Reconciler/applyPatch.lua b/plugin/src/Reconciler/applyPatch.lua index a625ebe9..4faa7310 100644 --- a/plugin/src/Reconciler/applyPatch.lua +++ b/plugin/src/Reconciler/applyPatch.lua @@ -5,6 +5,8 @@ Patches can come from the server or be generated by the client. ]] +local ChangeHistoryService = game:GetService("ChangeHistoryService") + local Packages = script.Parent.Parent.Parent.Packages local Log = require(Packages.Log) @@ -17,6 +19,8 @@ local reify = require(script.Parent.reify) local setProperty = require(script.Parent.setProperty) local function applyPatch(instanceMap, patch) + local patchTimestamp = DateTime.now():FormatLocalTime("LTS", "en-us") + -- Tracks any portions of the patch that could not be applied to the DOM. local unappliedPatch = PatchSet.newEmpty() @@ -199,6 +203,8 @@ local function applyPatch(instanceMap, patch) end end + ChangeHistoryService:SetWaypoint("Rojo: Patch " .. patchTimestamp) + return unappliedPatch end From fde78738b67e12d4f85b70534964749fb0732adc Mon Sep 17 00:00:00 2001 From: boatbomber Date: Wed, 5 Jul 2023 14:38:11 -0700 Subject: [PATCH 02/18] Improve sync info (#692) - Fixed an edge case where disconnecting and then reconnecting would retain outdated info - Fixed an issue where new patches wouldn't immediately update the change info text - Removed extraneous changes count info, as it was not useful and could be checked in the visualizer anyway - Added warning info for when some changes fail to apply - Updates timestamp of last sync even if patch was empty, to have a more accurate signal on Rojo's uptime --- CHANGELOG.md | 3 + plugin/src/App/StatusPages/Connected.lua | 98 ++++++++++++++++++------ plugin/src/App/init.lua | 51 +++++++----- plugin/src/PatchSet.lua | 15 +++- 4 files changed, 121 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 373eef77..17c7d27c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ * Added sync reminder notification. ([#689]) * Added protection against syncing a model to a place. ([#691]) * Fix Rojo breaking when users undo/redo in Studio ([#708]) +* Improved sync info text on Connected page. ([#692]) [#668]: https://github.com/rojo-rbx/rojo/pull/668 [#674]: https://github.com/rojo-rbx/rojo/pull/674 @@ -16,6 +17,8 @@ [#689]: https://github.com/rojo-rbx/rojo/pull/689 [#691]: https://github.com/rojo-rbx/rojo/pull/691 [#708]: https://github.com/rojo-rbx/rojo/pull/708 +[#692]: https://github.com/rojo-rbx/rojo/pull/692 + ## [7.3.0] - April 22, 2023 * Added `$attributes` to project format. ([#574]) diff --git a/plugin/src/App/StatusPages/Connected.lua b/plugin/src/App/StatusPages/Connected.lua index af43e6a2..ac152ef1 100644 --- a/plugin/src/App/StatusPages/Connected.lua +++ b/plugin/src/App/StatusPages/Connected.lua @@ -170,9 +170,56 @@ function ConnectedPage:getChangeInfoText() end local elapsed = os.time() - patchData.timestamp - local changes = PatchSet.countChanges(patchData.patch) + local unapplied = PatchSet.countChanges(patchData.unapplied) - return string.format("Synced %d change%s %s", changes, changes == 1 and "" or "s", timeSinceText(elapsed)) + return + "Synced " + .. timeSinceText(elapsed) + .. (if unapplied > 0 then + string.format( + ", but %d change%s failed to apply", + unapplied, + unapplied == 1 and "" or "s" + ) + else "") + .. "" +end + +function ConnectedPage:startChangeInfoTextUpdater() + -- Cancel any existing updater + self:stopChangeInfoTextUpdater() + + -- Start a new updater + self.changeInfoTextUpdater = task.defer(function() + while true do + if self.state.hoveringChangeInfo then + self.setChangeInfoText("" .. self:getChangeInfoText() .. "") + else + self.setChangeInfoText(self:getChangeInfoText()) + end + + local elapsed = os.time() - self.props.patchData.timestamp + local updateInterval = 1 + + -- Update timestamp text as frequently as currently needed + for _, UnitData in ipairs(AGE_UNITS) do + local UnitSeconds = UnitData[1] + if elapsed > UnitSeconds then + updateInterval = UnitSeconds + break + end + end + + task.wait(updateInterval) + end + end) +end + +function ConnectedPage:stopChangeInfoTextUpdater() + if self.changeInfoTextUpdater then + task.cancel(self.changeInfoTextUpdater) + self.changeInfoTextUpdater = nil + end end function ConnectedPage:init() @@ -195,34 +242,21 @@ function ConnectedPage:init() self:setState({ renderChanges = false, + hoveringChangeInfo = false, }) self.changeInfoText, self.setChangeInfoText = Roact.createBinding("") - self.changeInfoTextUpdater = task.defer(function() - while true do - self.setChangeInfoText(self:getChangeInfoText()) - - local elapsed = os.time() - self.props.patchData.timestamp - local updateInterval = 1 - - -- Update timestamp text as frequently as currently needed - for _, UnitData in ipairs(AGE_UNITS) do - local UnitSeconds = UnitData[1] - if elapsed >= UnitSeconds then - updateInterval = UnitSeconds - break - end - end - - task.wait(updateInterval) - end - end) + self:startChangeInfoTextUpdater() end function ConnectedPage:willUnmount() - if self.changeInfoTextUpdater then - task.cancel(self.changeInfoTextUpdater) + self:stopChangeInfoTextUpdater() +end + +function ConnectedPage:didUpdate(previousProps) + if self.props.patchData.timestamp ~= previousProps.patchData.timestamp then + self:startChangeInfoTextUpdater() end end @@ -271,6 +305,20 @@ function ConnectedPage:render() LayoutOrder = 3, BackgroundTransparency = 1, + [Roact.Event.MouseEnter] = function() + self:setState({ + hoveringChangeInfo = true, + }) + self.setChangeInfoText("" .. self:getChangeInfoText() .. "") + end, + + [Roact.Event.MouseLeave] = function() + self:setState({ + hoveringChangeInfo = false, + }) + self.setChangeInfoText(self:getChangeInfoText()) + end, + [Roact.Event.Activated] = function() if self.state.renderChanges then self.changeDrawerMotor:setGoal(Flipper.Spring.new(0, { @@ -284,6 +332,10 @@ function ConnectedPage:render() })) end end, + }, { + Tooltip = e(Tooltip.Trigger, { + text = if self.state.renderChanges then "Hide the changes" else "View the changes", + }), }), ChangesDrawer = e(ChangesDrawer, { diff --git a/plugin/src/App/init.lua b/plugin/src/App/init.lua index c7be7671..762f2bbd 100644 --- a/plugin/src/App/init.lua +++ b/plugin/src/App/init.lua @@ -122,6 +122,7 @@ function App:init() confirmData = {}, patchData = { patch = PatchSet.newEmpty(), + unapplied = PatchSet.newEmpty(), timestamp = os.time(), }, notifications = {}, @@ -357,35 +358,36 @@ function App:startSession() twoWaySync = sessionOptions.twoWaySync, }) - serveSession:onPatchApplied(function(patch, _unapplied) + serveSession:onPatchApplied(function(patch, unapplied) + local now = os.time() + local old = self.state.patchData + if PatchSet.isEmpty(patch) then - -- Ignore empty patches + -- Ignore empty patch, but update timestamp + self:setState({ + patchData = { + patch = old.patch, + unapplied = old.unapplied, + timestamp = now, + }, + }) return end - local now = os.time() - - local old = self.state.patchData if now - old.timestamp < 2 then -- Patches that apply in the same second are -- considered to be part of the same change for human clarity - local merged = PatchSet.newEmpty() - PatchSet.assign(merged, old.patch, patch) - - self:setState({ - patchData = { - patch = merged, - timestamp = now, - }, - }) - else - self:setState({ - patchData = { - patch = patch, - timestamp = now, - }, - }) + patch = PatchSet.assign(PatchSet.newEmpty(), old.patch, patch) + unapplied = PatchSet.assign(PatchSet.newEmpty(), old.unapplied, unapplied) end + + self:setState({ + patchData = { + patch = patch, + unapplied = unapplied, + timestamp = now, + }, + }) end) serveSession:onStatusChanged(function(status, details) @@ -409,6 +411,13 @@ function App:startSession() elseif status == ServeSession.Status.Disconnected then self.serveSession = nil self:releaseSyncLock() + self:setState({ + patchData = { + patch = PatchSet.newEmpty(), + unapplied = PatchSet.newEmpty(), + timestamp = os.time(), + }, + }) -- Details being present indicates that this -- disconnection was from an error. diff --git a/plugin/src/PatchSet.lua b/plugin/src/PatchSet.lua index c9f621dc..a9bebd80 100644 --- a/plugin/src/PatchSet.lua +++ b/plugin/src/PatchSet.lua @@ -212,13 +212,24 @@ function PatchSet.countChanges(patch) local count = 0 for _ in patch.added do + -- Adding an instance is 1 change count += 1 end for _ in patch.removed do + -- Removing an instance is 1 change count += 1 end - for _ in patch.updated do - count += 1 + for _, update in patch.updated do + -- Updating an instance is 1 change per property updated + for _ in update.changedProperties do + count += 1 + end + if update.changedName ~= nil then + count += 1 + end + if update.changedClassName ~= nil then + count += 1 + end end return count From 27357110b51e0d53b20c16b0606fdf4918d25a0c Mon Sep 17 00:00:00 2001 From: boatbomber Date: Sat, 8 Jul 2023 14:37:42 -0700 Subject: [PATCH 03/18] Handle strings and instances in patch removes for visualizer (#713) When an object is deleted in a patch, it is either represented with an ID or an Instance. On initial sync, removals are instances since the map does not contain those instances. Later removals of managed objects use an ID. The patch visualizer only handled instances, so this fixes that. Closes #710. --- plugin/src/App/Components/PatchVisualizer/init.lua | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/plugin/src/App/Components/PatchVisualizer/init.lua b/plugin/src/App/Components/PatchVisualizer/init.lua index 95673dfe..a751f2ff 100644 --- a/plugin/src/App/Components/PatchVisualizer/init.lua +++ b/plugin/src/App/Components/PatchVisualizer/init.lua @@ -7,6 +7,7 @@ local Packages = Rojo.Packages local Roact = require(Packages.Roact) local Log = require(Packages.Log) +local Types = require(Plugin.Types) local PatchSet = require(Plugin.PatchSet) local decodeValue = require(Plugin.Reconciler.decodeValue) local getProperty = require(Plugin.Reconciler.getProperty) @@ -240,7 +241,14 @@ function PatchVisualizer:buildTree(patch, instanceMap) }) end - for _, instance in patch.removed do + 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 = {} From a4fd53d5169f277d834b15b46800f379e0cf1a5d Mon Sep 17 00:00:00 2001 From: boatbomber Date: Sat, 8 Jul 2023 15:54:19 -0700 Subject: [PATCH 04/18] Update changelog to include #713 --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 17c7d27c..3680a84a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ * Added protection against syncing a model to a place. ([#691]) * Fix Rojo breaking when users undo/redo in Studio ([#708]) * Improved sync info text on Connected page. ([#692]) +* Fix patch visualizer breaking when instances are removed during sync ([#713]) [#668]: https://github.com/rojo-rbx/rojo/pull/668 [#674]: https://github.com/rojo-rbx/rojo/pull/674 @@ -18,6 +19,7 @@ [#691]: https://github.com/rojo-rbx/rojo/pull/691 [#708]: https://github.com/rojo-rbx/rojo/pull/708 [#692]: https://github.com/rojo-rbx/rojo/pull/692 +[#713]: https://github.com/rojo-rbx/rojo/pull/713 ## [7.3.0] - April 22, 2023 From 8139fdc738160c53550ebbbc96e5b88989bbdf05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bar=C4=B1=C5=9F?= <34089907+Barocena@users.noreply.github.com> Date: Sun, 9 Jul 2023 03:44:28 +0300 Subject: [PATCH 05/18] (feat) Add Changelog Check Action (#719) --- .github/workflows/ci.yml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 80346f50..fc018973 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -67,3 +67,19 @@ jobs: - name: Clippy run: cargo clippy + + Changelog: + name: Changelog Check + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - name: Changelog check + + uses: Zomzog/changelog-checker@v1.3.0 + with: + fileName: CHANGELOG.md + noChangelogLabel: no changelog + checkNotification: Simple + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + From ccc52b69d2f0edffb5299c2c37d097642e4359ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bar=C4=B1=C5=9F?= <34089907+Barocena@users.noreply.github.com> Date: Sun, 9 Jul 2023 04:30:59 +0300 Subject: [PATCH 06/18] (fix) Move changelog job to it's own workflow (#720) - moved the job to its own workflow file - changed the label to `skip changelog` --- .github/workflows/changelog.yml | 23 +++++++++++++++++++++++ .github/workflows/ci.yml | 15 --------------- 2 files changed, 23 insertions(+), 15 deletions(-) create mode 100644 .github/workflows/changelog.yml diff --git a/.github/workflows/changelog.yml b/.github/workflows/changelog.yml new file mode 100644 index 00000000..699152cc --- /dev/null +++ b/.github/workflows/changelog.yml @@ -0,0 +1,23 @@ +name: Changelog Check + +on: + pull_request: + types: [assigned, opened, synchronize, reopened, labeled, unlabeled] + branches: + - master + +jobs: + build: + name: Check Actions + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + + - name: Changelog check + uses: Zomzog/changelog-checker@v1.3.0 + with: + fileName: CHANGELOG.md + noChangelogLabel: skip changelog + checkNotification: Simple + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fc018973..b0e9fefe 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -68,18 +68,3 @@ jobs: - name: Clippy run: cargo clippy - Changelog: - name: Changelog Check - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v3 - - name: Changelog check - - uses: Zomzog/changelog-checker@v1.3.0 - with: - fileName: CHANGELOG.md - noChangelogLabel: no changelog - checkNotification: Simple - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - From 7ef4a1ff12e12f18eae60cf5e2a1e7c10bb0cd4a Mon Sep 17 00:00:00 2001 From: boatbomber Date: Sat, 8 Jul 2023 18:45:25 -0700 Subject: [PATCH 07/18] Select Instances from diff tree (#709) --- CHANGELOG.md | 2 + .../Components/PatchVisualizer/DomLabel.lua | 58 ++++++++++++++----- .../App/Components/PatchVisualizer/init.lua | 7 ++- 3 files changed, 51 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3680a84a..16c90c4b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ * Skip confirming patches that contain only a datamodel name change. ([#688]) * Added sync reminder notification. ([#689]) * Added protection against syncing a model to a place. ([#691]) +* Select Instances from diff tree view ([#709]) * Fix Rojo breaking when users undo/redo in Studio ([#708]) * Improved sync info text on Connected page. ([#692]) * Fix patch visualizer breaking when instances are removed during sync ([#713]) @@ -17,6 +18,7 @@ [#688]: https://github.com/rojo-rbx/rojo/pull/688 [#689]: https://github.com/rojo-rbx/rojo/pull/689 [#691]: https://github.com/rojo-rbx/rojo/pull/691 +[#709]: https://github.com/rojo-rbx/rojo/pull/709 [#708]: https://github.com/rojo-rbx/rojo/pull/708 [#692]: https://github.com/rojo-rbx/rojo/pull/692 [#713]: https://github.com/rojo-rbx/rojo/pull/713 diff --git a/plugin/src/App/Components/PatchVisualizer/DomLabel.lua b/plugin/src/App/Components/PatchVisualizer/DomLabel.lua index 9f967c32..351f8c31 100644 --- a/plugin/src/App/Components/PatchVisualizer/DomLabel.lua +++ b/plugin/src/App/Components/PatchVisualizer/DomLabel.lua @@ -1,3 +1,4 @@ +local SelectionService = game:GetService("Selection") local StudioService = game:GetService("StudioService") local Rojo = script:FindFirstAncestor("Rojo") @@ -14,6 +15,7 @@ local bindingUtil = require(Plugin.App.bindingUtil) local e = Roact.createElement local ChangeList = require(script.Parent.ChangeList) +local Tooltip = require(script.Parent.Parent.Tooltip) local Expansion = Roact.Component:extend("Expansion") @@ -106,21 +108,47 @@ function DomLabel:render() PaddingLeft = UDim.new(0, 10), PaddingRight = UDim.new(0, 10), }), - ExpandButton = if props.changeList - then e("TextButton", { - BackgroundTransparency = 1, - Text = "", - Size = UDim2.new(1, 0, 1, 0), - [Roact.Event.Activated] = function() - self.expanded = not self.expanded - local goalHeight = 30 + (if self.expanded then math.clamp(#self.props.changeList * 30, 30, 30 * 6) else 0) - self.motor:setGoal(Flipper.Spring.new(goalHeight, { - frequency = 5, - dampingRatio = 1, - })) - end, - }) - else nil, + Button = e("TextButton", { + BackgroundTransparency = 1, + Text = "", + Size = UDim2.new(1, 0, 1, 0), + [Roact.Event.Activated] = function(_rbx: Instance, _input: InputObject, clickCount: number) + if clickCount == 1 then + -- Double click opens the instance in explorer + self.lastDoubleClickTime = os.clock() + if props.instance then + SelectionService:Set({ props.instance }) + end + elseif clickCount == 0 then + -- Single click expands the changes + task.wait(0.25) + if os.clock() - (self.lastDoubleClickTime or 0) <= 0.25 then + -- This is a double click, so don't expand + return + end + + 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) + self.motor:setGoal(Flipper.Spring.new(goalHeight, { + frequency = 5, + dampingRatio = 1, + })) + end + end + end, + }, { + StateTip = if (props.instance or props.changeList) + then e(Tooltip.Trigger, { + text = (if props.changeList + then "Click to " .. (if self.expanded then "hide" else "view") .. " changes" + else "") .. (if props.instance + then (if props.changeList then " & d" else "D") .. "ouble click to open in Explorer" + else ""), + }) + else nil, + }), Expansion = if props.changeList then e(Expansion, { rendered = self.state.renderExpansion, diff --git a/plugin/src/App/Components/PatchVisualizer/init.lua b/plugin/src/App/Components/PatchVisualizer/init.lua index a751f2ff..7a3293e6 100644 --- a/plugin/src/App/Components/PatchVisualizer/init.lua +++ b/plugin/src/App/Components/PatchVisualizer/init.lua @@ -131,6 +131,7 @@ local function Tree() id = ancestorId, className = value.ClassName, name = value.Name, + instance = if typeof(value) == "Instance" then value else nil, }) previousId = ancestorId end @@ -236,6 +237,7 @@ function PatchVisualizer:buildTree(patch, instanceMap) patchType = "Edit", className = instance.ClassName, name = instance.Name, + instance = instance, hint = hint, changeList = changeList, }) @@ -271,10 +273,11 @@ function PatchVisualizer:buildTree(patch, instanceMap) patchType = "Remove", className = instance.ClassName, name = instance.Name, + instance = instance, }) end - for _, change in patch.added do + for id, change in patch.added do -- Gather ancestors from existing DOM or future additions local ancestry = {} local parentId = change.Parent @@ -344,6 +347,7 @@ function PatchVisualizer:buildTree(patch, instanceMap) name = change.Name, hint = hint, changeList = changeList, + instance = instanceMap.fromIds[id], }) end @@ -370,6 +374,7 @@ function PatchVisualizer:render() setElementHeight = setElementHeight, patchType = node.patchType, className = node.className, + instance = node.instance, name = node.name, hint = node.hint, changeList = node.changeList, From 0a932ff880f1683f91fd671c98b428c05973ec53 Mon Sep 17 00:00:00 2001 From: boatbomber Date: Sat, 8 Jul 2023 19:31:26 -0700 Subject: [PATCH 08/18] Show failed to apply in visualizer (#717) Objects with failed changes will highlight, and the specific failed changes in their list will highlight as well. --- CHANGELOG.md | 2 + .../Components/PatchVisualizer/ChangeList.lua | 12 ++++-- .../PatchVisualizer/DisplayValue.lua | 6 +-- .../Components/PatchVisualizer/DomLabel.lua | 4 +- .../App/Components/PatchVisualizer/init.lua | 38 +++++++++++++++---- plugin/src/App/StatusPages/Connected.lua | 2 + plugin/src/App/Theme.lua | 2 + 7 files changed, 49 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 16c90c4b..cc98fb55 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ * Fix Rojo breaking when users undo/redo in Studio ([#708]) * Improved sync info text on Connected page. ([#692]) * Fix patch visualizer breaking when instances are removed during sync ([#713]) +* Patch visualizer now indicates what changes failed to apply. ([#717]) [#668]: https://github.com/rojo-rbx/rojo/pull/668 [#674]: https://github.com/rojo-rbx/rojo/pull/674 @@ -22,6 +23,7 @@ [#708]: https://github.com/rojo-rbx/rojo/pull/708 [#692]: https://github.com/rojo-rbx/rojo/pull/692 [#713]: https://github.com/rojo-rbx/rojo/pull/713 +[#717]: https://github.com/rojo-rbx/rojo/pull/717 ## [7.3.0] - April 22, 2023 diff --git a/plugin/src/App/Components/PatchVisualizer/ChangeList.lua b/plugin/src/App/Components/PatchVisualizer/ChangeList.lua index beb0d327..b1fa54da 100644 --- a/plugin/src/App/Components/PatchVisualizer/ChangeList.lua +++ b/plugin/src/App/Components/PatchVisualizer/ChangeList.lua @@ -20,14 +20,13 @@ 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) return 0.93 + (0.07 * t) end) - local columnVisibility = props.columnVisibility - local rows = {} local pad = { PaddingLeft = UDim.new(0, 5), @@ -93,6 +92,9 @@ function ChangeList:render() continue -- Skip headers, already handled above end + local metadata = values[4] + local isWarning = metadata.isWarning + rows[row] = e("Frame", { Size = UDim2.new(1, 0, 0, 30), BackgroundTransparency = row % 2 ~= 0 and rowTransparency or 1, @@ -109,11 +111,11 @@ function ChangeList:render() }), A = e("TextLabel", { Visible = columnVisibility[1], - Text = tostring(values[1]), + Text = (if isWarning then "⚠ " else "") .. tostring(values[1]), BackgroundTransparency = 1, Font = Enum.Font.GothamMedium, TextSize = 14, - TextColor3 = theme.Settings.Setting.DescriptionColor, + TextColor3 = if isWarning then theme.Diff.Warning else theme.Settings.Setting.DescriptionColor, TextXAlignment = Enum.TextXAlignment.Left, TextTransparency = props.transparency, TextTruncate = Enum.TextTruncate.AtEnd, @@ -131,6 +133,7 @@ function ChangeList:render() e(DisplayValue, { value = values[2], transparency = props.transparency, + textColor = if isWarning then theme.Diff.Warning else theme.Settings.Setting.DescriptionColor, }) ), C = e( @@ -144,6 +147,7 @@ function ChangeList:render() e(DisplayValue, { value = values[3], transparency = props.transparency, + textColor = if isWarning then theme.Diff.Warning else theme.Settings.Setting.DescriptionColor, }) ), }) diff --git a/plugin/src/App/Components/PatchVisualizer/DisplayValue.lua b/plugin/src/App/Components/PatchVisualizer/DisplayValue.lua index 891ec9f2..3cf1a11a 100644 --- a/plugin/src/App/Components/PatchVisualizer/DisplayValue.lua +++ b/plugin/src/App/Components/PatchVisualizer/DisplayValue.lua @@ -34,7 +34,7 @@ local function DisplayValue(props) BackgroundTransparency = 1, Font = Enum.Font.GothamMedium, TextSize = 14, - TextColor3 = theme.Settings.Setting.DescriptionColor, + TextColor3 = props.textColor, TextXAlignment = Enum.TextXAlignment.Left, TextTransparency = props.transparency, TextTruncate = Enum.TextTruncate.AtEnd, @@ -78,7 +78,7 @@ local function DisplayValue(props) BackgroundTransparency = 1, Font = Enum.Font.GothamMedium, TextSize = 14, - TextColor3 = theme.Settings.Setting.DescriptionColor, + TextColor3 = props.textColor, TextXAlignment = Enum.TextXAlignment.Left, TextTransparency = props.transparency, TextTruncate = Enum.TextTruncate.AtEnd, @@ -95,7 +95,7 @@ local function DisplayValue(props) BackgroundTransparency = 1, Font = Enum.Font.GothamMedium, TextSize = 14, - TextColor3 = theme.Settings.Setting.DescriptionColor, + TextColor3 = props.textColor, TextXAlignment = Enum.TextXAlignment.Left, TextTransparency = props.transparency, TextTruncate = Enum.TextTruncate.AtEnd, diff --git a/plugin/src/App/Components/PatchVisualizer/DomLabel.lua b/plugin/src/App/Components/PatchVisualizer/DomLabel.lua index 351f8c31..98a4c0ff 100644 --- a/plugin/src/App/Components/PatchVisualizer/DomLabel.lua +++ b/plugin/src/App/Components/PatchVisualizer/DomLabel.lua @@ -180,7 +180,7 @@ function DomLabel:render() AnchorPoint = Vector2.new(0, 0.5), }), InstanceName = e("TextLabel", { - Text = props.name .. (props.hint and string.format( + Text = (if props.isWarning then "⚠ " else "") .. props.name .. (props.hint and string.format( ' %s', theme.AddressEntry.PlaceholderColor:ToHex(), props.hint @@ -189,7 +189,7 @@ function DomLabel:render() BackgroundTransparency = 1, Font = Enum.Font.GothamMedium, TextSize = 14, - TextColor3 = theme.Settings.Setting.DescriptionColor, + TextColor3 = if props.isWarning then theme.Diff.Warning else theme.Settings.Setting.DescriptionColor, TextXAlignment = Enum.TextXAlignment.Left, TextTransparency = props.transparency, TextTruncate = Enum.TextTruncate.AtEnd, diff --git a/plugin/src/App/Components/PatchVisualizer/init.lua b/plugin/src/App/Components/PatchVisualizer/init.lua index 7a3293e6..be1fc8c6 100644 --- a/plugin/src/App/Components/PatchVisualizer/init.lua +++ b/plugin/src/App/Components/PatchVisualizer/init.lua @@ -140,6 +140,15 @@ local function Tree() 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") @@ -160,7 +169,7 @@ function PatchVisualizer:shouldUpdate(nextProps) return not PatchSet.isEqual(currentPatch, nextPatch) end -function PatchVisualizer:buildTree(patch, instanceMap) +function PatchVisualizer:buildTree(patch, unappliedPatch, instanceMap) local tree = Tree() for _, change in patch.updated do @@ -184,13 +193,15 @@ function PatchVisualizer:buildTree(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?) + local function addProp(prop: string, current: any?, incoming: any?, metadata: any?) i += 1 hintBuffer[i] = prop - changeList[i] = { prop, current, incoming } + changeList[i] = { prop, current, incoming, metadata } end -- Gather the changes @@ -206,7 +217,10 @@ function PatchVisualizer:buildTree(patch, instanceMap) addProp( prop, if currentSuccess then currentValue else "[Error]", - if incomingSuccess then incomingValue else next(incoming) + if incomingSuccess then incomingValue else next(incoming), + { + isWarning = unappliedChanges[prop] ~= nil + } ) end @@ -303,6 +317,8 @@ function PatchVisualizer:buildTree(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 @@ -312,9 +328,13 @@ function PatchVisualizer:buildTree(patch, instanceMap) local success, incomingValue = decodeValue(incoming, instanceMap) if success then - table.insert(changeList, { prop, "N/A", incomingValue }) + table.insert(changeList, { prop, "N/A", incomingValue, { + isWarning = unappliedChanges[prop] ~= nil + } }) else - table.insert(changeList, { prop, "N/A", next(incoming) }) + table.insert(changeList, { prop, "N/A", next(incoming), { + isWarning = unappliedChanges[prop] ~= nil + } }) end end @@ -355,10 +375,11 @@ function PatchVisualizer:buildTree(patch, instanceMap) end function PatchVisualizer:render() - local patch = self.props.patch + 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, instanceMap) + local tree = self:buildTree(patch, unappliedPatch, instanceMap) -- Recusively draw tree local scrollElements, elementHeights = {}, {} @@ -374,6 +395,7 @@ function PatchVisualizer:render() setElementHeight = setElementHeight, patchType = node.patchType, className = node.className, + isWarning = next(findUnappliedPropsForId(unappliedPatch, node.id)) ~= nil, instance = node.instance, name = node.name, hint = node.hint, diff --git a/plugin/src/App/StatusPages/Connected.lua b/plugin/src/App/StatusPages/Connected.lua index ac152ef1..4c8ad548 100644 --- a/plugin/src/App/StatusPages/Connected.lua +++ b/plugin/src/App/StatusPages/Connected.lua @@ -84,6 +84,7 @@ function ChangesDrawer:render() columnVisibility = { true, false, true }, patch = self.props.patch, + unappliedPatch = self.props.unappliedPatch, instanceMap = self.serveSession.__instanceMap, }), }) @@ -342,6 +343,7 @@ function ConnectedPage:render() rendered = self.state.renderChanges, transparency = self.props.transparency, patch = self.props.patchData.patch, + unappliedPatch = self.props.patchData.unapplied, serveSession = self.props.serveSession, height = self.changeDrawerHeight, layoutOrder = 4, diff --git a/plugin/src/App/Theme.lua b/plugin/src/App/Theme.lua index 309045be..8ef05fab 100644 --- a/plugin/src/App/Theme.lua +++ b/plugin/src/App/Theme.lua @@ -100,6 +100,7 @@ local lightTheme = strict("LightTheme", { Remove = hexColor(0xffbdba), Edit = hexColor(0xbacdff), Row = hexColor(0x000000), + Warning = hexColor(0xFF8E3C), }, ConnectionDetails = { ProjectNameColor = hexColor(0x00000), @@ -195,6 +196,7 @@ local darkTheme = strict("DarkTheme", { Remove = hexColor(0x3F2D32), Edit = hexColor(0x193345), Row = hexColor(0xFFFFFF), + Warning = hexColor(0xFF8E3C), }, ConnectionDetails = { ProjectNameColor = hexColor(0xFFFFFF), From 7154113c13cd1fac3add676893343d0aa55afc15 Mon Sep 17 00:00:00 2001 From: boatbomber Date: Sat, 8 Jul 2023 21:23:25 -0700 Subject: [PATCH 09/18] Add buttons on connected page (#722) --- CHANGELOG.md | 3 +- plugin/src/App/StatusPages/Connected.lua | 61 ++++++++++++++++-------- plugin/src/App/init.lua | 10 +++- 3 files changed, 53 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cc98fb55..675db7f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ * Improved sync info text on Connected page. ([#692]) * Fix patch visualizer breaking when instances are removed during sync ([#713]) * Patch visualizer now indicates what changes failed to apply. ([#717]) +* Add buttons for navigation on the Connected page ([#722]) [#668]: https://github.com/rojo-rbx/rojo/pull/668 [#674]: https://github.com/rojo-rbx/rojo/pull/674 @@ -24,7 +25,7 @@ [#692]: https://github.com/rojo-rbx/rojo/pull/692 [#713]: https://github.com/rojo-rbx/rojo/pull/713 [#717]: https://github.com/rojo-rbx/rojo/pull/717 - +[#722]: https://github.com/rojo-rbx/rojo/pull/722 ## [7.3.0] - April 22, 2023 * Added `$attributes` to project format. ([#574]) diff --git a/plugin/src/App/StatusPages/Connected.lua b/plugin/src/App/StatusPages/Connected.lua index 4c8ad548..689b9022 100644 --- a/plugin/src/App/StatusPages/Connected.lua +++ b/plugin/src/App/StatusPages/Connected.lua @@ -12,6 +12,7 @@ local PatchSet = require(Plugin.PatchSet) local Header = require(Plugin.App.Components.Header) local IconButton = require(Plugin.App.Components.IconButton) +local TextButton = require(Plugin.App.Components.TextButton) local BorderedContainer = require(Plugin.App.Components.BorderedContainer) local Tooltip = require(Plugin.App.Components.Tooltip) local PatchVisualizer = require(Plugin.App.Components.PatchVisualizer) @@ -55,7 +56,7 @@ function ChangesDrawer:render() return e(BorderedContainer, { transparency = self.props.transparency, size = self.props.height:map(function(y) - return UDim2.new(1, 0, y, -180 * y) + return UDim2.new(1, 0, y, -220 * y) end), position = UDim2.new(0, 0, 1, 0), anchorPoint = Vector2.new(0, 1), @@ -138,22 +139,6 @@ local function ConnectionDetails(props) }), }), - Disconnect = e(IconButton, { - icon = Assets.Images.Icons.Close, - iconSize = 24, - color = theme.ConnectionDetails.DisconnectColor, - transparency = props.transparency, - - position = UDim2.new(1, 0, 0.5, 0), - anchorPoint = Vector2.new(1, 0.5), - - onClick = props.onDisconnect, - }, { - Tip = e(Tooltip.Trigger, { - text = "Disconnect from the Rojo sync server", - }), - }), - Padding = e("UIPadding", { PaddingLeft = UDim.new(0, 17), PaddingRight = UDim.new(0, 15), @@ -290,6 +275,44 @@ function ConnectedPage:render() onDisconnect = self.props.onDisconnect, }), + Buttons = e("Frame", { + Size = UDim2.new(1, 0, 0, 34), + LayoutOrder = 3, + BackgroundTransparency = 1, + ZIndex = 2, + }, { + Settings = e(TextButton, { + text = "Settings", + style = "Bordered", + transparency = self.props.transparency, + layoutOrder = 1, + onClick = self.props.onNavigateSettings, + }, { + Tip = e(Tooltip.Trigger, { + text = "View and modify plugin settings" + }), + }), + + Disconnect = e(TextButton, { + text = "Disconnect", + style = "Solid", + transparency = self.props.transparency, + layoutOrder = 2, + onClick = self.props.onDisconnect, + }, { + Tip = e(Tooltip.Trigger, { + text = "Disconnect from the Rojo sync server" + }), + }), + + Layout = e("UIListLayout", { + HorizontalAlignment = Enum.HorizontalAlignment.Right, + FillDirection = Enum.FillDirection.Horizontal, + SortOrder = Enum.SortOrder.LayoutOrder, + Padding = UDim.new(0, 10), + }), + }), + ChangeInfo = e("TextButton", { Text = self.changeInfoText, Font = Enum.Font.Gotham, @@ -303,7 +326,7 @@ function ConnectedPage:render() Size = UDim2.new(1, 0, 0, 28), - LayoutOrder = 3, + LayoutOrder = 4, BackgroundTransparency = 1, [Roact.Event.MouseEnter] = function() @@ -346,7 +369,7 @@ function ConnectedPage:render() unappliedPatch = self.props.patchData.unapplied, serveSession = self.props.serveSession, height = self.changeDrawerHeight, - layoutOrder = 4, + layoutOrder = 5, onClose = function() self.changeDrawerMotor:setGoal(Flipper.Spring.new(0, { diff --git a/plugin/src/App/init.lua b/plugin/src/App/init.lua index 762f2bbd..e569003a 100644 --- a/plugin/src/App/init.lua +++ b/plugin/src/App/init.lua @@ -563,6 +563,7 @@ function App:render() end, onNavigateSettings = function() + self.backPage = AppStatus.NotConnected self:setState({ appStatus = AppStatus.Settings, }) @@ -595,12 +596,19 @@ function App:render() onDisconnect = function() self:endSession() end, + + onNavigateSettings = function() + self.backPage = AppStatus.Connected + self:setState({ + appStatus = AppStatus.Settings, + }) + end, }), Settings = createPageElement(AppStatus.Settings, { onBack = function() self:setState({ - appStatus = AppStatus.NotConnected, + appStatus = self.backPage or AppStatus.NotConnected, }) end, }), From 623fa06d52a8aab01a2d267b7176186de55f1f87 Mon Sep 17 00:00:00 2001 From: boatbomber Date: Sat, 8 Jul 2023 21:41:47 -0700 Subject: [PATCH 10/18] Improve tooltip behavior (#723) --- CHANGELOG.md | 2 + plugin/src/App/Components/Tooltip.lua | 84 +++++++++++++++++++++------ 2 files changed, 68 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 675db7f8..856ed175 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ * Fix patch visualizer breaking when instances are removed during sync ([#713]) * Patch visualizer now indicates what changes failed to apply. ([#717]) * Add buttons for navigation on the Connected page ([#722]) +* Improve tooltip behavior ([#723]) [#668]: https://github.com/rojo-rbx/rojo/pull/668 [#674]: https://github.com/rojo-rbx/rojo/pull/674 @@ -26,6 +27,7 @@ [#713]: https://github.com/rojo-rbx/rojo/pull/713 [#717]: https://github.com/rojo-rbx/rojo/pull/717 [#722]: https://github.com/rojo-rbx/rojo/pull/722 +[#723]: https://github.com/rojo-rbx/rojo/pull/723 ## [7.3.0] - April 22, 2023 * Added `$attributes` to project format. ([#574]) diff --git a/plugin/src/App/Components/Tooltip.lua b/plugin/src/App/Components/Tooltip.lua index bc3443d5..aa88d9b6 100644 --- a/plugin/src/App/Components/Tooltip.lua +++ b/plugin/src/App/Components/Tooltip.lua @@ -165,9 +165,11 @@ function Trigger:init() self.id = HttpService:GenerateGUID(false) self.ref = Roact.createRef() self.mousePos = Vector2.zero + self.showingPopup = false self.destroy = function() self.props.context.removeTip(self.id) + self.showingPopup = false end end @@ -180,31 +182,77 @@ function Trigger:willUnmount() end end +function Trigger:didUpdate(prevProps) + if prevProps.text ~= self.props.text then + -- Any existing popup is now invalid + self.props.context.removeTip(self.id) + self.showingPopup = false + + -- Let the new text propagate + self:managePopup() + end +end + +function Trigger:isHovering() + local rbx = self.ref.current + if rbx then + local pos = rbx.AbsolutePosition + local size = rbx.AbsoluteSize + local mousePos = self.mousePos + + return + mousePos.X >= pos.X and mousePos.X <= pos.X + size.X + and mousePos.Y >= pos.Y and mousePos.Y <= pos.Y + size.Y + end + return false +end + +function Trigger:managePopup() + if self:isHovering() then + if self.showingPopup or self.showDelayThread then + -- Don't duplicate popups + return + end + + self.showDelayThread = task.delay(DELAY, function() + self.props.context.addTip(self.id, { + Text = self.props.text, + Position = self.mousePos, + Trigger = self.ref, + }) + self.showDelayThread = nil + self.showingPopup = true + end) + else + if self.showDelayThread then + task.cancel(self.showDelayThread) + self.showDelayThread = nil + end + self.props.context.removeTip(self.id) + self.showingPopup = false + end +end + function Trigger:render() + local function recalculate(rbx) + local widget = rbx:FindFirstAncestorOfClass("DockWidgetPluginGui") + if not widget then return end + self.mousePos = widget:GetRelativeMousePosition() + + self:managePopup() + end + return e("Frame", { Size = UDim2.fromScale(1, 1), BackgroundTransparency = 1, ZIndex = self.props.zIndex or 100, [Roact.Ref] = self.ref, - [Roact.Event.MouseMoved] = function(_rbx, x, y) - self.mousePos = Vector2.new(x, y) - end, - [Roact.Event.MouseEnter] = function() - self.showDelayThread = task.delay(DELAY, function() - self.props.context.addTip(self.id, { - Text = self.props.text, - Position = self.mousePos, - Trigger = self.ref, - }) - end) - end, - [Roact.Event.MouseLeave] = function() - if self.showDelayThread then - task.cancel(self.showDelayThread) - end - self.props.context.removeTip(self.id) - end, + [Roact.Change.AbsolutePosition] = recalculate, + [Roact.Change.AbsoluteSize] = recalculate, + [Roact.Event.MouseMoved] = recalculate, + [Roact.Event.MouseLeave] = recalculate, + [Roact.Event.MouseEnter] = recalculate, }) end From 80eb14f9da6d24268d34ad46eab793a486816fae Mon Sep 17 00:00:00 2001 From: utrain <63367489+u-train@users.noreply.github.com> Date: Wed, 12 Jul 2023 13:00:09 -0400 Subject: [PATCH 11/18] Removed InstanceSnapshot snapshot_id's redudant Ref. (#730) Ref now is an optional inside, so it's redundant to have an option wrapping an option. The only snapshots that were changed were any that had a Ref within (from none to zeroed). Some also had some newlines added in the end. --- src/snapshot/instance_snapshot.rs | 9 ++++----- src/snapshot/patch_apply.rs | 4 ++-- src/snapshot/patch_compute.rs | 14 ++++++++------ src/snapshot/tests/compute.rs | 13 +++++++------ ...brojo__snapshot__tests__compute__add_child.snap | 3 ++- src/snapshot_middleware/json_model.rs | 4 ++-- src/snapshot_middleware/project.rs | 4 ++-- ...apshot_middleware__csv__test__csv_from_vfs.snap | 3 +-- ...pshot_middleware__csv__test__csv_with_meta.snap | 3 +-- ...apshot_middleware__dir__test__empty_folder.snap | 3 +-- ...ot_middleware__dir__test__folder_in_folder.snap | 5 ++--- ..._middleware__json__test__instance_from_vfs.snap | 3 +-- ...ddleware__json_model__test__model_from_vfs.snap | 5 ++--- ...e__json_model__test__model_from_vfs_legacy.snap | 5 ++--- ...hot_middleware__lua__test__client_from_vfs.snap | 3 +-- ...hot_middleware__lua__test__module_from_vfs.snap | 3 +-- ...ot_middleware__lua__test__module_with_meta.snap | 3 +-- ...hot_middleware__lua__test__script_disabled.snap | 3 +-- ...ot_middleware__lua__test__script_with_meta.snap | 3 +-- ...hot_middleware__lua__test__server_from_vfs.snap | 3 +-- ...e__project__test__project_from_direct_file.snap | 3 ++- ...ect__test__project_path_property_overrides.snap | 3 +-- ...ware__project__test__project_with_children.snap | 5 +++-- ...roject__test__project_with_path_to_project.snap | 3 ++- ...project_with_path_to_project_with_children.snap | 5 +++-- ...e__project__test__project_with_path_to_txt.snap | 3 +-- ...ct__test__project_with_resolved_properties.snap | 3 +-- ...__test__project_with_unresolved_properties.snap | 3 +-- ...t_middleware__txt__test__instance_from_vfs.snap | 3 +-- 29 files changed, 58 insertions(+), 69 deletions(-) diff --git a/src/snapshot/instance_snapshot.rs b/src/snapshot/instance_snapshot.rs index 36c51821..0b3a4fca 100644 --- a/src/snapshot/instance_snapshot.rs +++ b/src/snapshot/instance_snapshot.rs @@ -17,9 +17,8 @@ use super::InstanceMetadata; // - Replace use of Variant with a sum of Variant + borrowed value #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct InstanceSnapshot { - // FIXME: Don't use Option anymore! /// A temporary ID applied to the snapshot that's used for Ref properties. - pub snapshot_id: Option, + pub snapshot_id: Ref, /// Rojo-specific metadata associated with the instance. pub metadata: InstanceMetadata, @@ -42,7 +41,7 @@ pub struct InstanceSnapshot { impl InstanceSnapshot { pub fn new() -> Self { Self { - snapshot_id: None, + snapshot_id: Ref::none(), metadata: InstanceMetadata::default(), name: Cow::Borrowed("DEFAULT"), class_name: Cow::Borrowed("DEFAULT"), @@ -88,7 +87,7 @@ impl InstanceSnapshot { } } - pub fn snapshot_id(self, snapshot_id: Option) -> Self { + pub fn snapshot_id(self, snapshot_id: Ref) -> Self { Self { snapshot_id, ..self @@ -120,7 +119,7 @@ impl InstanceSnapshot { .collect(); Self { - snapshot_id: Some(id), + snapshot_id: id, metadata: InstanceMetadata::default(), name: Cow::Owned(instance.name), class_name: Cow::Owned(instance.class), diff --git a/src/snapshot/patch_apply.rs b/src/snapshot/patch_apply.rs index 5818776c..572b8455 100644 --- a/src/snapshot/patch_apply.rs +++ b/src/snapshot/patch_apply.rs @@ -135,7 +135,7 @@ fn apply_add_child( context.has_refs_to_rewrite.insert(id); } - if let Some(snapshot_id) = snapshot_id { + if snapshot_id.is_some() { context.snapshot_id_to_instance_id.insert(snapshot_id, id); } @@ -231,7 +231,7 @@ mod test { let root_id = tree.get_root_id(); let snapshot = InstanceSnapshot { - snapshot_id: None, + snapshot_id: Ref::none(), metadata: Default::default(), name: Cow::Borrowed("Foo"), class_name: Cow::Borrowed("Bar"), diff --git a/src/snapshot/patch_compute.rs b/src/snapshot/patch_compute.rs index f5d5a67b..5d7abc69 100644 --- a/src/snapshot/patch_compute.rs +++ b/src/snapshot/patch_compute.rs @@ -77,8 +77,10 @@ fn compute_patch_set_internal( id: Ref, patch_set: &mut PatchSet, ) { - if let Some(snapshot_id) = snapshot.snapshot_id { - context.snapshot_id_to_instance_id.insert(snapshot_id, id); + if snapshot.snapshot_id.is_some() { + context + .snapshot_id_to_instance_id + .insert(snapshot.snapshot_id, id); } let instance = tree @@ -244,7 +246,7 @@ mod test { // addition of a prop named Self, which is a self-referential Ref. let snapshot_id = Ref::new(); let snapshot = InstanceSnapshot { - snapshot_id: Some(snapshot_id), + snapshot_id: snapshot_id, properties: hashmap! { "Self".to_owned() => Variant::Ref(snapshot_id), }, @@ -286,13 +288,13 @@ mod test { // This patch describes the existing instance with a new child added. let snapshot_id = Ref::new(); let snapshot = InstanceSnapshot { - snapshot_id: Some(snapshot_id), + snapshot_id: snapshot_id, children: vec![InstanceSnapshot { properties: hashmap! { "Self".to_owned() => Variant::Ref(snapshot_id), }, - snapshot_id: None, + snapshot_id: Ref::none(), metadata: Default::default(), name: Cow::Borrowed("child"), class_name: Cow::Borrowed("child"), @@ -311,7 +313,7 @@ mod test { added_instances: vec![PatchAdd { parent_id: root_id, instance: InstanceSnapshot { - snapshot_id: None, + snapshot_id: Ref::none(), metadata: Default::default(), properties: hashmap! { "Self".to_owned() => Variant::Ref(root_id), diff --git a/src/snapshot/tests/compute.rs b/src/snapshot/tests/compute.rs index 309d73b6..fc6a0b63 100644 --- a/src/snapshot/tests/compute.rs +++ b/src/snapshot/tests/compute.rs @@ -3,6 +3,7 @@ use std::borrow::Cow; use insta::assert_yaml_snapshot; use maplit::hashmap; +use rbx_dom_weak::types::Ref; use rojo_insta_ext::RedactionMap; use crate::snapshot::{compute_patch_set, InstanceSnapshot, RojoTree}; @@ -15,7 +16,7 @@ fn set_name_and_class_name() { redactions.intern(tree.get_root_id()); let snapshot = InstanceSnapshot { - snapshot_id: None, + snapshot_id: Ref::none(), metadata: Default::default(), name: Cow::Borrowed("Some Folder"), class_name: Cow::Borrowed("Folder"), @@ -37,7 +38,7 @@ fn set_property() { redactions.intern(tree.get_root_id()); let snapshot = InstanceSnapshot { - snapshot_id: None, + snapshot_id: Ref::none(), metadata: Default::default(), name: Cow::Borrowed("ROOT"), class_name: Cow::Borrowed("ROOT"), @@ -70,7 +71,7 @@ fn remove_property() { } let snapshot = InstanceSnapshot { - snapshot_id: None, + snapshot_id: Ref::none(), metadata: Default::default(), name: Cow::Borrowed("ROOT"), class_name: Cow::Borrowed("ROOT"), @@ -92,13 +93,13 @@ fn add_child() { redactions.intern(tree.get_root_id()); let snapshot = InstanceSnapshot { - snapshot_id: None, + snapshot_id: Ref::none(), metadata: Default::default(), name: Cow::Borrowed("ROOT"), class_name: Cow::Borrowed("ROOT"), properties: Default::default(), children: vec![InstanceSnapshot { - snapshot_id: None, + snapshot_id: Ref::none(), metadata: Default::default(), name: Cow::Borrowed("New"), class_name: Cow::Borrowed("Folder"), @@ -131,7 +132,7 @@ fn remove_child() { } let snapshot = InstanceSnapshot { - snapshot_id: None, + snapshot_id: Ref::none(), metadata: Default::default(), name: Cow::Borrowed("ROOT"), class_name: Cow::Borrowed("ROOT"), diff --git a/src/snapshot/tests/snapshots/librojo__snapshot__tests__compute__add_child.snap b/src/snapshot/tests/snapshots/librojo__snapshot__tests__compute__add_child.snap index 97bd639e..c4695b7c 100644 --- a/src/snapshot/tests/snapshots/librojo__snapshot__tests__compute__add_child.snap +++ b/src/snapshot/tests/snapshots/librojo__snapshot__tests__compute__add_child.snap @@ -6,7 +6,7 @@ removed_instances: [] added_instances: - parent_id: id-1 instance: - snapshot_id: ~ + snapshot_id: "00000000000000000000000000000000" metadata: ignore_unknown_instances: false relevant_paths: [] @@ -16,3 +16,4 @@ added_instances: properties: {} children: [] updated_instances: [] + diff --git a/src/snapshot_middleware/json_model.rs b/src/snapshot_middleware/json_model.rs index 0f1388e4..b38c47a7 100644 --- a/src/snapshot_middleware/json_model.rs +++ b/src/snapshot_middleware/json_model.rs @@ -2,7 +2,7 @@ use std::{borrow::Cow, collections::HashMap, path::Path, str}; use anyhow::Context; use memofs::Vfs; -use rbx_dom_weak::types::Attributes; +use rbx_dom_weak::types::{Attributes, Ref}; use serde::Deserialize; use crate::{ @@ -112,7 +112,7 @@ impl JsonModel { } Ok(InstanceSnapshot { - snapshot_id: None, + snapshot_id: Ref::none(), metadata: Default::default(), name: Cow::Owned(name), class_name: Cow::Owned(class_name), diff --git a/src/snapshot_middleware/project.rs b/src/snapshot_middleware/project.rs index f8fa7920..2d54a179 100644 --- a/src/snapshot_middleware/project.rs +++ b/src/snapshot_middleware/project.rs @@ -2,7 +2,7 @@ use std::{borrow::Cow, collections::HashMap, path::Path}; use anyhow::{bail, Context}; use memofs::Vfs; -use rbx_dom_weak::types::Attributes; +use rbx_dom_weak::types::{Attributes, Ref}; use rbx_reflection::ClassTag; use crate::{ @@ -271,7 +271,7 @@ pub fn snapshot_project_node( )); Ok(Some(InstanceSnapshot { - snapshot_id: None, + snapshot_id: Ref::none(), name, class_name, properties, diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__csv__test__csv_from_vfs.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__csv__test__csv_from_vfs.snap index 7a685efb..11a8d81c 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__csv__test__csv_from_vfs.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__csv__test__csv_from_vfs.snap @@ -1,9 +1,8 @@ --- source: src/snapshot_middleware/csv.rs expression: instance_snapshot - --- -snapshot_id: ~ +snapshot_id: "00000000000000000000000000000000" metadata: ignore_unknown_instances: false instigating_source: diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__csv__test__csv_with_meta.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__csv__test__csv_with_meta.snap index 70bae90d..eda82606 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__csv__test__csv_with_meta.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__csv__test__csv_with_meta.snap @@ -1,9 +1,8 @@ --- source: src/snapshot_middleware/csv.rs expression: instance_snapshot - --- -snapshot_id: ~ +snapshot_id: "00000000000000000000000000000000" metadata: ignore_unknown_instances: true instigating_source: diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__dir__test__empty_folder.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__dir__test__empty_folder.snap index 948dda0b..e59da8db 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__dir__test__empty_folder.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__dir__test__empty_folder.snap @@ -1,9 +1,8 @@ --- source: src/snapshot_middleware/dir.rs -assertion_line: 127 expression: instance_snapshot --- -snapshot_id: ~ +snapshot_id: "00000000000000000000000000000000" metadata: ignore_unknown_instances: false instigating_source: diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__dir__test__folder_in_folder.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__dir__test__folder_in_folder.snap index cf07b0ed..8c2ee9cc 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__dir__test__folder_in_folder.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__dir__test__folder_in_folder.snap @@ -1,9 +1,8 @@ --- source: src/snapshot_middleware/dir.rs -assertion_line: 148 expression: instance_snapshot --- -snapshot_id: ~ +snapshot_id: "00000000000000000000000000000000" metadata: ignore_unknown_instances: false instigating_source: @@ -23,7 +22,7 @@ name: foo class_name: Folder properties: {} children: - - snapshot_id: ~ + - snapshot_id: "00000000000000000000000000000000" metadata: ignore_unknown_instances: false instigating_source: diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json__test__instance_from_vfs.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json__test__instance_from_vfs.snap index 75329b85..b1cca7cb 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json__test__instance_from_vfs.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json__test__instance_from_vfs.snap @@ -1,9 +1,8 @@ --- source: src/snapshot_middleware/json.rs expression: instance_snapshot - --- -snapshot_id: ~ +snapshot_id: "00000000000000000000000000000000" metadata: ignore_unknown_instances: false instigating_source: diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json_model__test__model_from_vfs.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json_model__test__model_from_vfs.snap index a6a0743e..5d4db1fc 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json_model__test__model_from_vfs.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json_model__test__model_from_vfs.snap @@ -1,9 +1,8 @@ --- source: src/snapshot_middleware/json_model.rs expression: instance_snapshot - --- -snapshot_id: ~ +snapshot_id: "00000000000000000000000000000000" metadata: ignore_unknown_instances: false instigating_source: @@ -17,7 +16,7 @@ properties: Value: Int64: 5 children: - - snapshot_id: ~ + - snapshot_id: "00000000000000000000000000000000" metadata: ignore_unknown_instances: false relevant_paths: [] diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json_model__test__model_from_vfs_legacy.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json_model__test__model_from_vfs_legacy.snap index a58fdb0f..5d4db1fc 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json_model__test__model_from_vfs_legacy.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__json_model__test__model_from_vfs_legacy.snap @@ -1,9 +1,8 @@ --- source: src/snapshot_middleware/json_model.rs -assertion_line: 186 expression: instance_snapshot --- -snapshot_id: ~ +snapshot_id: "00000000000000000000000000000000" metadata: ignore_unknown_instances: false instigating_source: @@ -17,7 +16,7 @@ properties: Value: Int64: 5 children: - - snapshot_id: ~ + - snapshot_id: "00000000000000000000000000000000" metadata: ignore_unknown_instances: false relevant_paths: [] diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__client_from_vfs.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__client_from_vfs.snap index f774f561..40b4bbe2 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__client_from_vfs.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__client_from_vfs.snap @@ -1,9 +1,8 @@ --- source: src/snapshot_middleware/lua.rs expression: instance_snapshot - --- -snapshot_id: ~ +snapshot_id: "00000000000000000000000000000000" metadata: ignore_unknown_instances: false instigating_source: diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__module_from_vfs.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__module_from_vfs.snap index ac9ac72a..6877e9a9 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__module_from_vfs.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__module_from_vfs.snap @@ -1,9 +1,8 @@ --- source: src/snapshot_middleware/lua.rs expression: instance_snapshot - --- -snapshot_id: ~ +snapshot_id: "00000000000000000000000000000000" metadata: ignore_unknown_instances: false instigating_source: diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__module_with_meta.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__module_with_meta.snap index f623955f..8dcc57a9 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__module_with_meta.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__module_with_meta.snap @@ -1,9 +1,8 @@ --- source: src/snapshot_middleware/lua.rs expression: instance_snapshot - --- -snapshot_id: ~ +snapshot_id: "00000000000000000000000000000000" metadata: ignore_unknown_instances: true instigating_source: diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__script_disabled.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__script_disabled.snap index 8a9b4776..3f040877 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__script_disabled.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__script_disabled.snap @@ -1,9 +1,8 @@ --- source: src/snapshot_middleware/lua.rs expression: instance_snapshot - --- -snapshot_id: ~ +snapshot_id: "00000000000000000000000000000000" metadata: ignore_unknown_instances: false instigating_source: diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__script_with_meta.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__script_with_meta.snap index 1234b48a..6bdd2a38 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__script_with_meta.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__script_with_meta.snap @@ -1,9 +1,8 @@ --- source: src/snapshot_middleware/lua.rs expression: instance_snapshot - --- -snapshot_id: ~ +snapshot_id: "00000000000000000000000000000000" metadata: ignore_unknown_instances: true instigating_source: diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__server_from_vfs.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__server_from_vfs.snap index 76473c75..63d33a0a 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__server_from_vfs.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__lua__test__server_from_vfs.snap @@ -1,9 +1,8 @@ --- source: src/snapshot_middleware/lua.rs expression: instance_snapshot - --- -snapshot_id: ~ +snapshot_id: "00000000000000000000000000000000" metadata: ignore_unknown_instances: false instigating_source: diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_from_direct_file.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_from_direct_file.snap index 42393420..795b14c1 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_from_direct_file.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_from_direct_file.snap @@ -2,7 +2,7 @@ source: src/snapshot_middleware/project.rs expression: instance_snapshot --- -snapshot_id: ~ +snapshot_id: "00000000000000000000000000000000" metadata: ignore_unknown_instances: true instigating_source: @@ -14,3 +14,4 @@ name: direct-project class_name: Model properties: {} children: [] + diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_path_property_overrides.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_path_property_overrides.snap index 49b3b506..7427c9dc 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_path_property_overrides.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_path_property_overrides.snap @@ -1,9 +1,8 @@ --- source: src/snapshot_middleware/project.rs expression: instance_snapshot - --- -snapshot_id: ~ +snapshot_id: "00000000000000000000000000000000" metadata: ignore_unknown_instances: true instigating_source: diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_children.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_children.snap index ccf6b535..b50eb1be 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_children.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_children.snap @@ -2,7 +2,7 @@ source: src/snapshot_middleware/project.rs expression: instance_snapshot --- -snapshot_id: ~ +snapshot_id: "00000000000000000000000000000000" metadata: ignore_unknown_instances: true instigating_source: @@ -14,7 +14,7 @@ name: children class_name: Folder properties: {} children: - - snapshot_id: ~ + - snapshot_id: "00000000000000000000000000000000" metadata: ignore_unknown_instances: true instigating_source: @@ -29,3 +29,4 @@ children: class_name: Model properties: {} children: [] + diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_project.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_project.snap index a520b7c7..d64a46e7 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_project.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_project.snap @@ -2,7 +2,7 @@ source: src/snapshot_middleware/project.rs expression: instance_snapshot --- -snapshot_id: ~ +snapshot_id: "00000000000000000000000000000000" metadata: ignore_unknown_instances: true instigating_source: @@ -15,3 +15,4 @@ name: path-project class_name: Model properties: {} children: [] + diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_project_with_children.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_project_with_children.snap index c71b4d2d..0ceb043a 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_project_with_children.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_project_with_children.snap @@ -2,7 +2,7 @@ source: src/snapshot_middleware/project.rs expression: instance_snapshot --- -snapshot_id: ~ +snapshot_id: "00000000000000000000000000000000" metadata: ignore_unknown_instances: true instigating_source: @@ -15,7 +15,7 @@ name: path-child-project class_name: Folder properties: {} children: - - snapshot_id: ~ + - snapshot_id: "00000000000000000000000000000000" metadata: ignore_unknown_instances: true instigating_source: @@ -30,3 +30,4 @@ children: class_name: Model properties: {} children: [] + diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_txt.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_txt.snap index 38b797c3..51a222f9 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_txt.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_path_to_txt.snap @@ -1,9 +1,8 @@ --- source: src/snapshot_middleware/project.rs expression: instance_snapshot - --- -snapshot_id: ~ +snapshot_id: "00000000000000000000000000000000" metadata: ignore_unknown_instances: false instigating_source: diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_resolved_properties.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_resolved_properties.snap index 29a23a07..9b8e91ef 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_resolved_properties.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_resolved_properties.snap @@ -1,9 +1,8 @@ --- source: src/snapshot_middleware/project.rs expression: instance_snapshot - --- -snapshot_id: ~ +snapshot_id: "00000000000000000000000000000000" metadata: ignore_unknown_instances: true instigating_source: diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_unresolved_properties.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_unresolved_properties.snap index a422e9f8..a07300bb 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_unresolved_properties.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__project_with_unresolved_properties.snap @@ -1,9 +1,8 @@ --- source: src/snapshot_middleware/project.rs expression: instance_snapshot - --- -snapshot_id: ~ +snapshot_id: "00000000000000000000000000000000" metadata: ignore_unknown_instances: true instigating_source: diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__txt__test__instance_from_vfs.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__txt__test__instance_from_vfs.snap index ad469490..b8a8d4af 100644 --- a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__txt__test__instance_from_vfs.snap +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__txt__test__instance_from_vfs.snap @@ -1,9 +1,8 @@ --- source: src/snapshot_middleware/txt.rs expression: instance_snapshot - --- -snapshot_id: ~ +snapshot_id: "00000000000000000000000000000000" metadata: ignore_unknown_instances: false instigating_source: From 28d48a76e34811b4385afea1a570de3ba561447c Mon Sep 17 00:00:00 2001 From: boatbomber Date: Wed, 12 Jul 2023 10:01:21 -0700 Subject: [PATCH 12/18] Fix tooltips crashing on thread cancellation edge cases (#727) --- plugin/src/App/Components/Tooltip.lua | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugin/src/App/Components/Tooltip.lua b/plugin/src/App/Components/Tooltip.lua index aa88d9b6..9c1fb151 100644 --- a/plugin/src/App/Components/Tooltip.lua +++ b/plugin/src/App/Components/Tooltip.lua @@ -175,7 +175,7 @@ end function Trigger:willUnmount() if self.showDelayThread then - task.cancel(self.showDelayThread) + pcall(task.cancel, self.showDelayThread) end if self.destroy then self.destroy() @@ -225,7 +225,7 @@ function Trigger:managePopup() end) else if self.showDelayThread then - task.cancel(self.showDelayThread) + pcall(task.cancel, self.showDelayThread) self.showDelayThread = nil end self.props.context.removeTip(self.id) From 9d48af2b505d5043927ab2ad9c6ff09995ae47c7 Mon Sep 17 00:00:00 2001 From: boatbomber Date: Wed, 12 Jul 2023 13:00:07 -0700 Subject: [PATCH 13/18] Better settings control (#725) - Lock some settings during sync - Make experimental features more clearly labelled --- CHANGELOG.md | 2 ++ plugin/src/App/Components/Checkbox.lua | 13 +++++++++---- plugin/src/App/Components/Dropdown.lua | 12 ++++++++++-- plugin/src/App/StatusPages/Settings/Setting.lua | 11 ++++++++--- plugin/src/App/StatusPages/Settings/init.lua | 8 ++++++-- plugin/src/App/init.lua | 2 ++ plugin/src/Assets.lua | 2 ++ 7 files changed, 39 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 856ed175..e36fb581 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ * Patch visualizer now indicates what changes failed to apply. ([#717]) * Add buttons for navigation on the Connected page ([#722]) * Improve tooltip behavior ([#723]) +* Better settings controls ([#725]) [#668]: https://github.com/rojo-rbx/rojo/pull/668 [#674]: https://github.com/rojo-rbx/rojo/pull/674 @@ -28,6 +29,7 @@ [#717]: https://github.com/rojo-rbx/rojo/pull/717 [#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 ## [7.3.0] - April 22, 2023 * Added `$attributes` to project format. ([#574]) diff --git a/plugin/src/App/Components/Checkbox.lua b/plugin/src/App/Components/Checkbox.lua index 21ec56b3..aa351b1f 100644 --- a/plugin/src/App/Components/Checkbox.lua +++ b/plugin/src/App/Components/Checkbox.lua @@ -51,10 +51,15 @@ function Checkbox:render() ZIndex = self.props.zIndex, BackgroundTransparency = 1, - [Roact.Event.Activated] = self.props.onClick, + [Roact.Event.Activated] = function() + if self.props.locked then return end + self.props.onClick() + end, }, { StateTip = e(Tooltip.Trigger, { - text = if self.props.active then "Enabled" else "Disabled", + text = + (if self.props.locked then "[LOCKED] " else "") + .. (if self.props.active then "Enabled" else "Disabled"), }), Active = e(SlicedImage, { @@ -65,7 +70,7 @@ function Checkbox:render() zIndex = 2, }, { Icon = e("ImageLabel", { - Image = Assets.Images.Checkbox.Active, + Image = if self.props.locked then Assets.Images.Checkbox.Locked else Assets.Images.Checkbox.Active, ImageColor3 = theme.Active.IconColor, ImageTransparency = activeTransparency, @@ -84,7 +89,7 @@ function Checkbox:render() size = UDim2.new(1, 0, 1, 0), }, { Icon = e("ImageLabel", { - Image = Assets.Images.Checkbox.Inactive, + Image = if self.props.locked then Assets.Images.Checkbox.Locked else Assets.Images.Checkbox.Inactive, ImageColor3 = theme.Inactive.IconColor, ImageTransparency = self.props.transparency, diff --git a/plugin/src/App/Components/Dropdown.lua b/plugin/src/App/Components/Dropdown.lua index ef9a4211..21a60908 100644 --- a/plugin/src/App/Components/Dropdown.lua +++ b/plugin/src/App/Components/Dropdown.lua @@ -29,7 +29,13 @@ function Dropdown:init() }) end -function Dropdown:didUpdate() +function Dropdown:didUpdate(prevProps) + if self.props.locked and not prevProps.locked then + self:setState({ + open = false, + }) + end + self.openMotor:setGoal( Flipper.Spring.new(self.state.open and 1 or 0, { frequency = 6, @@ -68,6 +74,7 @@ function Dropdown:render() Font = Enum.Font.GothamMedium, [Roact.Event.Activated] = function() + if self.props.locked then return end self:setState({ open = false, }) @@ -89,6 +96,7 @@ function Dropdown:render() BackgroundTransparency = 1, [Roact.Event.Activated] = function() + if self.props.locked then return end self:setState({ open = not self.state.open, }) @@ -101,7 +109,7 @@ function Dropdown:render() size = UDim2.new(1, 0, 1, 0), }, { DropArrow = e("ImageLabel", { - Image = Assets.Images.Dropdown.Arrow, + Image = if self.props.locked then Assets.Images.Dropdown.Locked else Assets.Images.Dropdown.Arrow, ImageColor3 = self.openBinding:map(function(a) return theme.Closed.IconColor:Lerp(theme.Open.IconColor, a) end), diff --git a/plugin/src/App/StatusPages/Settings/Setting.lua b/plugin/src/App/StatusPages/Settings/Setting.lua index 0c0de9f4..79e83e23 100644 --- a/plugin/src/App/StatusPages/Settings/Setting.lua +++ b/plugin/src/App/StatusPages/Settings/Setting.lua @@ -67,6 +67,7 @@ function Setting:render() }, { Input = if self.props.options ~= nil then e(Dropdown, { + locked = self.props.locked, options = self.props.options, active = self.state.setting, transparency = self.props.transparency, @@ -78,6 +79,7 @@ function Setting:render() }) else e(Checkbox, { + locked = self.props.locked, active = self.state.setting, transparency = self.props.transparency, position = UDim2.new(1, 0, 0.5, 0), @@ -106,12 +108,13 @@ function Setting:render() BackgroundTransparency = 1, }, { Name = e("TextLabel", { - Text = self.props.name, + Text = (if self.props.experimental then "⚠ " else "") .. self.props.name, Font = Enum.Font.GothamBold, TextSize = 17, TextColor3 = theme.Setting.NameColor, TextXAlignment = Enum.TextXAlignment.Left, TextTransparency = self.props.transparency, + RichText = true, Size = UDim2.new(1, 0, 0, 17), @@ -120,7 +123,7 @@ function Setting:render() }), Description = e("TextLabel", { - Text = self.props.description, + Text = (if self.props.experimental then "[Experimental] " else "") .. self.props.description, Font = Enum.Font.Gotham, LineHeight = 1.2, TextSize = 14, @@ -128,11 +131,13 @@ function Setting:render() TextXAlignment = Enum.TextXAlignment.Left, TextTransparency = self.props.transparency, TextWrapped = true, + RichText = true, Size = self.containerSize:map(function(value) + local desc = (if self.props.experimental then "[Experimental] " else "") .. self.props.description local offset = (self.props.onReset and 34 or 0) + (self.props.options ~= nil and 120 or 40) local textBounds = getTextBounds( - self.props.description, 14, Enum.Font.Gotham, 1.2, + desc, 14, Enum.Font.Gotham, 1.2, Vector2.new(value.X - offset, math.huge) ) return UDim2.new(1, -offset, 0, textBounds.Y) diff --git a/plugin/src/App/StatusPages/Settings/init.lua b/plugin/src/App/StatusPages/Settings/init.lua index f83e145c..298e39a4 100644 --- a/plugin/src/App/StatusPages/Settings/init.lua +++ b/plugin/src/App/StatusPages/Settings/init.lua @@ -115,7 +115,9 @@ function SettingsPage:render() OpenScriptsExternally = e(Setting, { id = "openScriptsExternally", name = "Open Scripts Externally", - description = "EXPERIMENTAL! Attempt to open scripts in an external editor", + description = "Attempt to open scripts in an external editor", + locked = self.props.syncActive, + experimental = true, transparency = self.props.transparency, layoutOrder = 4, }), @@ -123,7 +125,9 @@ function SettingsPage:render() TwoWaySync = e(Setting, { id = "twoWaySync", name = "Two-Way Sync", - description = "EXPERIMENTAL! Editing files in Studio will sync them into the filesystem", + description = "Editing files in Studio will sync them into the filesystem", + locked = self.props.syncActive, + experimental = true, transparency = self.props.transparency, layoutOrder = 5, }), diff --git a/plugin/src/App/init.lua b/plugin/src/App/init.lua index e569003a..fd534d0c 100644 --- a/plugin/src/App/init.lua +++ b/plugin/src/App/init.lua @@ -606,6 +606,8 @@ function App:render() }), Settings = createPageElement(AppStatus.Settings, { + syncActive = self.serveSession ~= nil and self.serveSession:getStatus() == ServeSession.Status.Connected, + onBack = function() self:setState({ appStatus = self.backPage or AppStatus.NotConnected, diff --git a/plugin/src/Assets.lua b/plugin/src/Assets.lua index 3c919b54..020ec3eb 100644 --- a/plugin/src/Assets.lua +++ b/plugin/src/Assets.lua @@ -33,9 +33,11 @@ local Assets = { Checkbox = { Active = "rbxassetid://6016251644", Inactive = "rbxassetid://6016251963", + Locked = "rbxassetid://14011257320", }, Dropdown = { Arrow = "rbxassetid://10131770538", + Locked = "rbxassetid://14011257320", }, Spinner = { Foreground = "rbxassetid://3222731032", From 6e40993199b9f7891bef11a095f3acb5086c8f2b Mon Sep 17 00:00:00 2001 From: boatbomber Date: Thu, 13 Jul 2023 20:09:19 -0700 Subject: [PATCH 14/18] 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 e36fb581..130175e7 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 b1fa54da..ed6da37a 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 98a4c0ff..facf70cd 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 be1fc8c6..73af4502 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) + if self.props.patchTree ~= nextProps.patchTree then + return true + end + 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, - }) + if currentPatch ~= nil or nextPatch ~= nil then + return not PatchSet.isEqual(currentPatch, nextPatch) 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, - }) - 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], - }) - 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) + 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 - end - for _, node in alphabeticalPairs(tree.ROOT.children) do - drawNode(node, 0) + + 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 3359eaf6..a5592347 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 689b9022..00f6822b 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 fd534d0c..23d0a506 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 00000000..b241472d --- /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 1db295a4..12945782 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 From 6e320b1fd563100623ea117f6014d1e8a4ee80c9 Mon Sep 17 00:00:00 2001 From: Shae Date: Fri, 14 Jul 2023 15:36:50 -0500 Subject: [PATCH 15/18] Add support for TOML files (#633) TOML maps well to Lua, is easier to read and write than JSON, and is commonly used by Roblox tools. Use cases: * Put game, plugin, or library config in a toml file * Sync in toml files generated by tools * Sync in config files for tools so that the game can double-check that the config file has been followed. (e.g. check that packages match versions specified in wally.toml) --- CHANGELOG.md | 2 + Cargo.lock | 1 + Cargo.toml | 1 + src/snapshot_middleware/mod.rs | 4 + ...leware__toml__test__instance_from_vfs.snap | 20 +++ src/snapshot_middleware/toml.rs | 123 ++++++++++++++++++ 6 files changed, 151 insertions(+) create mode 100644 src/snapshot_middleware/snapshots/librojo__snapshot_middleware__toml__test__instance_from_vfs.snap create mode 100644 src/snapshot_middleware/toml.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 130175e7..2c2e1bbf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ * Improve tooltip behavior ([#723]) * Better settings controls ([#725]) * Rework patch visualizer with many fixes and improvements ([#726]) +* Added support for syncing in `.toml` files ([#633]) [#668]: https://github.com/rojo-rbx/rojo/pull/668 [#674]: https://github.com/rojo-rbx/rojo/pull/674 @@ -32,6 +33,7 @@ [#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 +[#633]: https://github.com/rojo-rbx/rojo/pull/633 ## [7.3.0] - April 22, 2023 * Added `$attributes` to project format. ([#574]) diff --git a/Cargo.lock b/Cargo.lock index 12e9f860..fab34b07 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1922,6 +1922,7 @@ dependencies = [ "termcolor", "thiserror", "tokio", + "toml", "tracy-client 0.13.2", "uuid", "walkdir", diff --git a/Cargo.toml b/Cargo.toml index bbbbcbf9..124d5c8f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -80,6 +80,7 @@ ritz = "0.1.0" roblox_install = "1.0.0" serde = { version = "1.0.130", features = ["derive", "rc"] } serde_json = "1.0.68" +toml = "0.5.9" termcolor = "1.1.2" thiserror = "1.0.30" tokio = { version = "1.12.0", features = ["rt", "rt-multi-thread"] } diff --git a/src/snapshot_middleware/mod.rs b/src/snapshot_middleware/mod.rs index 3d5de76b..c690a223 100644 --- a/src/snapshot_middleware/mod.rs +++ b/src/snapshot_middleware/mod.rs @@ -14,6 +14,7 @@ mod meta_file; mod project; mod rbxm; mod rbxmx; +mod toml; mod txt; mod util; @@ -32,6 +33,7 @@ use self::{ project::snapshot_project, rbxm::snapshot_rbxm, rbxmx::snapshot_rbxmx, + toml::snapshot_toml, txt::snapshot_txt, util::PathExt, }; @@ -117,6 +119,8 @@ pub fn snapshot_from_vfs( return Ok(None); } else if path.file_name_ends_with(".json") { return snapshot_json(context, vfs, path); + } else if path.file_name_ends_with(".toml") { + return snapshot_toml(context, vfs, path); } else if let Ok(name) = csv_name { match name { // init csv are handled elsewhere and should not turn into diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__toml__test__instance_from_vfs.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__toml__test__instance_from_vfs.snap new file mode 100644 index 00000000..3529bf89 --- /dev/null +++ b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__toml__test__instance_from_vfs.snap @@ -0,0 +1,20 @@ +--- +source: src/snapshot_middleware/toml.rs +expression: instance_snapshot +--- +snapshot_id: "00000000000000000000000000000000" +metadata: + ignore_unknown_instances: false + instigating_source: + Path: /foo.toml + relevant_paths: + - /foo.toml + - /foo.meta.json + context: {} +name: foo +class_name: ModuleScript +properties: + Source: + String: "return {\n\t[\"1invalidident\"] = \"nice\",\n\tarray = {1, 2, 3},\n\tdates = {\n\t\tlocaldate = \"1979-05-27\",\n\t\tlocaldatetime = \"1979-05-27T07:32:00\",\n\t\tlocaltime = \"00:32:00.999999\",\n\t\toffset1 = \"1979-05-27T00:32:00.999999-07:00\",\n\t\toffset2 = \"1979-05-27T07:32:00Z\",\n\t},\n\t[\"false\"] = false,\n\tfloat = 1234.5452,\n\tint = 1234,\n\tobject = {\n\t\thello = \"world\",\n\t},\n\t[\"true\"] = true,\n}" +children: [] + diff --git a/src/snapshot_middleware/toml.rs b/src/snapshot_middleware/toml.rs new file mode 100644 index 00000000..6ee9be47 --- /dev/null +++ b/src/snapshot_middleware/toml.rs @@ -0,0 +1,123 @@ +use std::path::Path; + +use anyhow::Context; +use maplit::hashmap; +use memofs::{IoResultExt, Vfs}; + +use crate::{ + lua_ast::{Expression, Statement}, + snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot}, +}; + +use super::{meta_file::AdjacentMetadata, util::PathExt}; + +pub fn snapshot_toml( + context: &InstanceContext, + vfs: &Vfs, + path: &Path, +) -> anyhow::Result> { + let name = path.file_name_trim_end(".toml")?; + let contents = vfs.read(path)?; + + let value: toml::Value = toml::from_slice(&contents) + .with_context(|| format!("File contains malformed TOML: {}", path.display()))?; + + let as_lua = toml_to_lua(value).to_string(); + + let properties = hashmap! { + "Source".to_owned() => as_lua.into(), + }; + + let meta_path = path.with_file_name(format!("{}.meta.json", name)); + + let mut snapshot = InstanceSnapshot::new() + .name(name) + .class_name("ModuleScript") + .properties(properties) + .metadata( + InstanceMetadata::new() + .instigating_source(path) + .relevant_paths(vec![path.to_path_buf(), meta_path.clone()]) + .context(context), + ); + + if let Some(meta_contents) = vfs.read(&meta_path).with_not_found()? { + let mut metadata = AdjacentMetadata::from_slice(&meta_contents, meta_path)?; + metadata.apply_all(&mut snapshot)?; + } + + Ok(Some(snapshot)) +} + +fn toml_to_lua(value: toml::Value) -> Statement { + Statement::Return(toml_to_lua_value(value)) +} + +fn toml_to_lua_value(value: toml::Value) -> Expression { + use toml::Value; + + match value { + Value::Datetime(value) => Expression::String(value.to_string()), + Value::Boolean(value) => Expression::Bool(value), + Value::Float(value) => Expression::Number(value), + Value::Integer(value) => Expression::Number(value as f64), + Value::String(value) => Expression::String(value), + Value::Array(values) => { + Expression::Array(values.into_iter().map(toml_to_lua_value).collect()) + } + Value::Table(values) => Expression::table( + values + .into_iter() + .map(|(key, value)| (key.into(), toml_to_lua_value(value))) + .collect(), + ), + } +} + +#[cfg(test)] +mod test { + use super::*; + + use memofs::{InMemoryFs, VfsSnapshot}; + + #[test] + fn instance_from_vfs() { + let mut imfs = InMemoryFs::new(); + imfs.load_snapshot( + "/foo.toml", + VfsSnapshot::file( + r#" + array = [1, 2, 3] + true = true + false = false + int = 1234 + float = 1234.5452 + "1invalidident" = "nice" + + [object] + hello = "world" + + [dates] + offset1 = 1979-05-27T00:32:00.999999-07:00 + offset2 = 1979-05-27 07:32:00Z + localdatetime = 1979-05-27T07:32:00 + localdate = 1979-05-27 + localtime = 00:32:00.999999 + "#, + ), + ) + .unwrap(); + + let mut vfs = Vfs::new(imfs.clone()); + + let instance_snapshot = snapshot_toml( + &InstanceContext::default(), + &mut vfs, + Path::new("/foo.toml"), + ) + .unwrap() + .unwrap(); + + insta::assert_yaml_snapshot!(instance_snapshot); + } +} From dd01a9bef33a47865d92f194b823899ea33cd7da Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 14 Jul 2023 14:04:11 -0700 Subject: [PATCH 16/18] Bump openssl from 0.10.51 to 0.10.55 (#737) --- Cargo.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fab34b07..cc544cf7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1308,9 +1308,9 @@ dependencies = [ [[package]] name = "openssl" -version = "0.10.51" +version = "0.10.55" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "97ea2d98598bf9ada7ea6ee8a30fb74f9156b63bbe495d64ec2b87c269d2dda3" +checksum = "345df152bc43501c5eb9e4654ff05f794effb78d4efe3d53abc158baddc0703d" dependencies = [ "bitflags", "cfg-if 1.0.0", @@ -1349,9 +1349,9 @@ dependencies = [ [[package]] name = "openssl-sys" -version = "0.9.86" +version = "0.9.90" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "992bac49bdbab4423199c654a5515bd2a6c6a23bf03f2dd3bdb7e5ae6259bc69" +checksum = "374533b0e45f3a7ced10fcaeccca020e66656bc03dac384f852e4e5a7a8104a6" dependencies = [ "cc", "libc", From 8662d2227ca057f42c9b8f588f8436406362e020 Mon Sep 17 00:00:00 2001 From: boatbomber Date: Sun, 16 Jul 2023 14:59:30 -0700 Subject: [PATCH 17/18] Improve Rojo sync reliability (#739) Uses non-recursive main loop and avoids hanging promises --- plugin/src/ApiContext.lua | 15 ++---------- plugin/src/ServeSession.lua | 49 +++++++++++++++++++++++++------------ 2 files changed, 35 insertions(+), 29 deletions(-) diff --git a/plugin/src/ApiContext.lua b/plugin/src/ApiContext.lua index 98890d4f..9a683dd5 100644 --- a/plugin/src/ApiContext.lua +++ b/plugin/src/ApiContext.lua @@ -11,13 +11,6 @@ local validateApiInfo = Types.ifEnabled(Types.ApiInfoResponse) local validateApiRead = Types.ifEnabled(Types.ApiReadResponse) local validateApiSubscribe = Types.ifEnabled(Types.ApiSubscribeResponse) ---[[ - Returns a promise that will never resolve nor reject. -]] -local function hangingPromise() - return Promise.new(function() end) -end - local function rejectFailedRequests(response) if response.code >= 400 then local message = string.format("HTTP %s:\n%s", tostring(response.code), response.body) @@ -212,12 +205,8 @@ function ApiContext:retrieveMessages() local function sendRequest() local request = Http.get(url) :catch(function(err) - if err.type == Http.Error.Kind.Timeout then - if self.__connected then - return sendRequest() - else - return hangingPromise() - end + if err.type == Http.Error.Kind.Timeout and self.__connected then + return sendRequest() end return Promise.reject(err) diff --git a/plugin/src/ServeSession.lua b/plugin/src/ServeSession.lua index 08dc4041..4e5116c8 100644 --- a/plugin/src/ServeSession.lua +++ b/plugin/src/ServeSession.lua @@ -299,27 +299,44 @@ function ServeSession:__initialSync(serverInfo) end function ServeSession:__mainSyncLoop() - return self.__apiContext:retrieveMessages() - :andThen(function(messages) - Log.trace("Serve session {} retrieved {} messages", tostring(self), #messages) + return Promise.new(function(resolve, reject) + while self.__status == Status.Connected do + local success, result = self.__apiContext:retrieveMessages() + :andThen(function(messages) + if self.__status == Status.Disconnected then + -- In the time it took to retrieve messages, we disconnected + -- so we just resolve immediately without patching anything + return + end - for _, message in ipairs(messages) do - local unappliedPatch = self.__reconciler:applyPatch(message) + Log.trace("Serve session {} retrieved {} messages", tostring(self), #messages) - if not PatchSet.isEmpty(unappliedPatch) then - Log.warn("Could not apply all changes requested by the Rojo server:\n{}", - PatchSet.humanSummary(self.__instanceMap, unappliedPatch)) - end + for _, message in messages do + local unappliedPatch = self.__reconciler:applyPatch(message) - if self.__patchAppliedCallback then - pcall(self.__patchAppliedCallback, message, unappliedPatch) - end + if not PatchSet.isEmpty(unappliedPatch) then + Log.warn("Could not apply all changes requested by the Rojo server:\n{}", + PatchSet.humanSummary(self.__instanceMap, unappliedPatch)) + end + + if self.__patchAppliedCallback then + pcall(self.__patchAppliedCallback, message, unappliedPatch) + end + end + end):await() + + if self.__status == Status.Disconnected then + -- If we are no longer connected after applying, we stop silently + -- without checking for errors as they are no longer relevant + break + elseif success == false then + reject(result) end + end - if self.__status ~= Status.Disconnected then - return self:__mainSyncLoop() - end - end) + -- We are no longer connected, so we resolve the promise + resolve() + end) end function ServeSession:__stopInternal(err) From 49154778234d90aeb47be260c42a6f9d7b62fb1f Mon Sep 17 00:00:00 2001 From: boatbomber Date: Sun, 16 Jul 2023 21:03:06 -0700 Subject: [PATCH 18/18] Expose reconciler hooks on servesession (#741) --- plugin/src/App/init.lua | 2 +- plugin/src/ServeSession.lua | 16 ++++++---------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/plugin/src/App/init.lua b/plugin/src/App/init.lua index 23d0a506..f4c30f3c 100644 --- a/plugin/src/App/init.lua +++ b/plugin/src/App/init.lua @@ -374,7 +374,7 @@ function App:startSession() end) end) - serveSession:onPatchApplied(function(patch, unapplied) + serveSession:hookPostcommit(function(patch, _instanceMap, unapplied) local now = os.time() local old = self.state.patchData diff --git a/plugin/src/ServeSession.lua b/plugin/src/ServeSession.lua index 4e5116c8..6a3fcdc4 100644 --- a/plugin/src/ServeSession.lua +++ b/plugin/src/ServeSession.lua @@ -97,7 +97,6 @@ function ServeSession.new(options) __instanceMap = instanceMap, __changeBatcher = changeBatcher, __statusChangedCallback = nil, - __patchAppliedCallback = nil, __connections = connections, } @@ -129,8 +128,12 @@ function ServeSession:setConfirmCallback(callback) self.__userConfirmCallback = callback end -function ServeSession:onPatchApplied(callback) - self.__patchAppliedCallback = callback +function ServeSession:hookPrecommit(callback) + return self.__reconciler:hookPrecommit(callback) +end + +function ServeSession:hookPostcommit(callback) + return self.__reconciler:hookPostcommit(callback) end function ServeSession:start() @@ -291,9 +294,6 @@ function ServeSession:__initialSync(serverInfo) Log.warn("Could not apply all changes requested by the Rojo server:\n{}", PatchSet.humanSummary(self.__instanceMap, unappliedPatch)) end - if self.__patchAppliedCallback then - pcall(self.__patchAppliedCallback, catchUpPatch, unappliedPatch) - end end end) end @@ -318,10 +318,6 @@ function ServeSession:__mainSyncLoop() Log.warn("Could not apply all changes requested by the Rojo server:\n{}", PatchSet.humanSummary(self.__instanceMap, unappliedPatch)) end - - if self.__patchAppliedCallback then - pcall(self.__patchAppliedCallback, message, unappliedPatch) - end end end):await()