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

@@ -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])

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("<i>Synced %d change%s %s</i>", changes, changes == 1 and "" or "s", timeSinceText(elapsed))
+	return
+		"<i>Synced "
+		.. timeSinceText(elapsed)
+		.. (if unapplied > 0 then
+			string.format(
+				", <font color=\"#FF8E3C\">but %d change%s failed to apply</font>",
+				unapplied,
+				unapplied == 1 and "" or "s"
+			)
+		else "")
+		.. "</i>"
+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("<u>" .. self:getChangeInfoText() .. "</u>")
+			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("<u>" .. self:getChangeInfoText() .. "</u>")
+				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, {

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.

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