From 6b0f7f94b60d05e4413fca3c6d7fa47bce6b257d Mon Sep 17 00:00:00 2001
From: boatbomber <zack@boatbomber.com>
Date: Thu, 1 Jun 2023 11:18:04 -0400
Subject: [PATCH 01/16] Fix disconnected session activity (#675)

When a session is disconnected, the apiContext long-polling for messages
continues until resolved/rejected. This means that even after a session
is disconnected, a message can be received and handled.

This leads to bad behavior, as the session was already cleaned up and
the message cannot be handled correctly. The instance map was cleaned up
upon disconnect, so it will warn about unapplied changes to unknown
instances. (Like #512)

It's very easy to repro:
Connect a session, disconnect it, then save a change.


https://github.com/rojo-rbx/rojo/assets/40185666/846a7269-7043-4727-9f9c-b3ac55a18a3a

-----------

This PR fixes that neatly by tracking all active requests in a map, and
cancelling their promises when we disconnect.
---
 plugin/src/ApiContext.lua   | 17 ++++++++++++++++-
 plugin/src/ServeSession.lua |  2 ++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/plugin/src/ApiContext.lua b/plugin/src/ApiContext.lua
index 78f54fed..98890d4f 100644
--- a/plugin/src/ApiContext.lua
+++ b/plugin/src/ApiContext.lua
@@ -93,6 +93,7 @@ function ApiContext.new(baseUrl)
 		__sessionId = nil,
 		__messageCursor = -1,
 		__connected = true,
+		__activeRequests = {},
 	}
 
 	return setmetatable(self, ApiContext)
@@ -113,6 +114,11 @@ end
 
 function ApiContext:disconnect()
 	self.__connected = false
+	for request in self.__activeRequests do
+		Log.trace("Cancelling request {}", request)
+		request:cancel()
+	end
+	self.__activeRequests = {}
 end
 
 function ApiContext:setMessageCursor(index)
@@ -204,7 +210,7 @@ function ApiContext:retrieveMessages()
 	local url = ("%s/api/subscribe/%s"):format(self.__baseUrl, self.__messageCursor)
 
 	local function sendRequest()
-		return Http.get(url)
+		local request = Http.get(url)
 			:catch(function(err)
 				if err.type == Http.Error.Kind.Timeout then
 					if self.__connected then
@@ -216,6 +222,15 @@ function ApiContext:retrieveMessages()
 
 				return Promise.reject(err)
 			end)
+
+		Log.trace("Tracking request {}", request)
+		self.__activeRequests[request] = true
+
+		return request:finally(function(...)
+			Log.trace("Cleaning up request {}", request)
+			self.__activeRequests[request] = nil
+			return ...
+		end)
 	end
 
 	return sendRequest()
diff --git a/plugin/src/ServeSession.lua b/plugin/src/ServeSession.lua
index 12b9ffae..f77c8c74 100644
--- a/plugin/src/ServeSession.lua
+++ b/plugin/src/ServeSession.lua
@@ -285,6 +285,8 @@ end
 function ServeSession:__mainSyncLoop()
 	return self.__apiContext:retrieveMessages()
 		:andThen(function(messages)
+			Log.trace("Serve session {} retrieved {} messages", tostring(self), #messages)
+
 			for _, message in ipairs(messages) do
 				local unappliedPatch = self.__reconciler:applyPatch(message)
 

From 65423043409e5fe35e513c4ccec8ff0ae7845c1e Mon Sep 17 00:00:00 2001
From: boatbomber <zack@boatbomber.com>
Date: Sun, 4 Jun 2023 01:46:16 -0400
Subject: [PATCH 02/16] Fix the diff visualizer of connected sessions (#674)

Clicking on the "X changes X ago" message opens up a handy diff
visualizer to see what those changes were. However, it had quite a few
issues that needed fixing.

- Disconnecting a session with it expanded caused an error as it tried
to read the serveSession that no longer exists during the page fade
transition. (#671)
- Resolved by converting to stateful component and holding the
serveSession during the lifetime to ensure it can render the last known
changes during the fade transition
- Leaving it open while new changes are synced did not update the
visualizer
- The patch data was piggybacking on an existing binding, which meant
that new patches did not trigger rerender.
    - Resolved by converting to state
    - Also made some improvements to that old binding
- Moved from app to connected page for better organization and
separation of duties
      - No more useless updates causing rerenders with no real change
- Scroll window child component wouldn't actually display the updated
visuals
    - Resolved by making major improvements to VirtualScroller
      - Made more robust against edge case states
      - Made smarter about knowing when it needs to refresh

As you can see in this slow motion GIF, it works now.

![slowedDemo](https://github.com/rojo-rbx/rojo/assets/40185666/c9fc8489-72a9-47be-ae45-9c420e1535d4)
---
 .../Components/PatchVisualizer/DomLabel.lua   |  8 +-
 plugin/src/App/Components/VirtualScroller.lua | 14 ++-
 plugin/src/App/StatusPages/Connected.lua      | 94 +++++++++++++------
 plugin/src/App/init.lua                       | 38 ++++----
 4 files changed, 98 insertions(+), 56 deletions(-)

diff --git a/plugin/src/App/Components/PatchVisualizer/DomLabel.lua b/plugin/src/App/Components/PatchVisualizer/DomLabel.lua
index 8d282ab9..9f967c32 100644
--- a/plugin/src/App/Components/PatchVisualizer/DomLabel.lua
+++ b/plugin/src/App/Components/PatchVisualizer/DomLabel.lua
@@ -40,11 +40,6 @@ end
 local DomLabel = Roact.Component:extend("DomLabel")
 
 function DomLabel:init()
-	self.maxElementHeight = 0
-	if self.props.changeList then
-		self.maxElementHeight = math.clamp(#self.props.changeList * 30, 30, 30 * 6)
-	end
-
 	local initHeight = self.props.elementHeight:getValue()
 	self.expanded = initHeight > 30
 
@@ -118,7 +113,8 @@ function DomLabel:render()
 					Size = UDim2.new(1, 0, 1, 0),
 					[Roact.Event.Activated] = function()
 						self.expanded = not self.expanded
-						self.motor:setGoal(Flipper.Spring.new((self.expanded and self.maxElementHeight or 0) + 30, {
+						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,
 						}))
diff --git a/plugin/src/App/Components/VirtualScroller.lua b/plugin/src/App/Components/VirtualScroller.lua
index 2922a74f..69bcf5f1 100644
--- a/plugin/src/App/Components/VirtualScroller.lua
+++ b/plugin/src/App/Components/VirtualScroller.lua
@@ -99,18 +99,30 @@ function VirtualScroller:refresh()
 	})
 end
 
+function VirtualScroller:didUpdate(previousProps)
+	if self.props.count ~= previousProps.count then
+		-- Items have changed, so we need to refresh
+		self:refresh()
+	end
+end
+
 function VirtualScroller:render()
 	local props, state = self.props, self.state
 
 	local items = {}
 	for i = state.Start, state.End do
+		local content = props.render(i)
+		if content == nil then
+			continue
+		end
+
 		items["Item" .. i] = e("Frame", {
 			LayoutOrder = i,
 			Size = props.getHeightBinding(i):map(function(height)
 				return UDim2.new(1, 0, 0, height)
 			end),
 			BackgroundTransparency = 1,
-		}, props.render(i))
+		}, content)
 	end
 
 	return Theme.with(function(theme)
diff --git a/plugin/src/App/StatusPages/Connected.lua b/plugin/src/App/StatusPages/Connected.lua
index 91484926..af43e6a2 100644
--- a/plugin/src/App/StatusPages/Connected.lua
+++ b/plugin/src/App/StatusPages/Connected.lua
@@ -26,11 +26,11 @@ function timeSinceText(elapsed: number): string
 
 	local ageText = string.format("%d seconds ago", elapsed)
 
-	for _,UnitData in ipairs(AGE_UNITS) do
+	for _, UnitData in ipairs(AGE_UNITS) do
 		local UnitSeconds, UnitName = UnitData[1], UnitData[2]
 		if elapsed > UnitSeconds then
-			local c = math.floor(elapsed/UnitSeconds)
-			ageText = string.format("%d %s%s ago", c, UnitName, c>1 and "s" or "")
+			local c = math.floor(elapsed / UnitSeconds)
+			ageText = string.format("%d %s%s ago", c, UnitName, c > 1 and "s" or "")
 			break
 		end
 	end
@@ -38,45 +38,53 @@ function timeSinceText(elapsed: number): string
 	return ageText
 end
 
-local function ChangesDrawer(props)
-	if props.rendered == false then
+local ChangesDrawer = Roact.Component:extend("ConnectedPage")
+
+function ChangesDrawer:init()
+	-- Hold onto the serve session during the lifecycle of this component
+	-- so that it can still render during the fade out after disconnecting
+	self.serveSession = self.props.serveSession
+end
+
+function ChangesDrawer:render()
+	if self.props.rendered == false or self.serveSession == nil then
 		return nil
 	end
 
 	return Theme.with(function(theme)
 		return e(BorderedContainer, {
-			transparency = props.transparency,
-			size = props.height:map(function(y)
+			transparency = self.props.transparency,
+			size = self.props.height:map(function(y)
 				return UDim2.new(1, 0, y, -180 * y)
 			end),
 			position = UDim2.new(0, 0, 1, 0),
 			anchorPoint = Vector2.new(0, 1),
-			layoutOrder = props.layoutOrder,
+			layoutOrder = self.props.layoutOrder,
 		}, {
 			Close = e(IconButton, {
 				icon = Assets.Images.Icons.Close,
 				iconSize = 24,
 				color = theme.ConnectionDetails.DisconnectColor,
-				transparency = props.transparency,
+				transparency = self.props.transparency,
 
 				position = UDim2.new(1, 0, 0, 0),
 				anchorPoint = Vector2.new(1, 0),
 
-				onClick = props.onClose,
+				onClick = self.props.onClose,
 			}, {
 				Tip = e(Tooltip.Trigger, {
-					text = "Close the patch visualizer"
+					text = "Close the patch visualizer",
 				}),
 			}),
 
 			PatchVisualizer = e(PatchVisualizer, {
 				size = UDim2.new(1, 0, 1, 0),
-				transparency = props.transparency,
+				transparency = self.props.transparency,
 				layoutOrder = 3,
 
-				columnVisibility = {true, false, true},
-				patch = props.patchInfo:getValue().patch,
-				instanceMap = props.serveSession.__instanceMap,
+				columnVisibility = { true, false, true },
+				patch = self.props.patch,
+				instanceMap = self.serveSession.__instanceMap,
 			}),
 		})
 	end)
@@ -91,7 +99,7 @@ local function ConnectionDetails(props)
 		}, {
 			TextContainer = e("Frame", {
 				Size = UDim2.new(1, 0, 1, 0),
-				BackgroundTransparency = 1
+				BackgroundTransparency = 1,
 			}, {
 				ProjectName = e("TextLabel", {
 					Text = props.projectName,
@@ -141,7 +149,7 @@ local function ConnectionDetails(props)
 				onClick = props.onDisconnect,
 			}, {
 				Tip = e(Tooltip.Trigger, {
-					text = "Disconnect from the Rojo sync server"
+					text = "Disconnect from the Rojo sync server",
 				}),
 			}),
 
@@ -155,6 +163,18 @@ end
 
 local ConnectedPage = Roact.Component:extend("ConnectedPage")
 
+function ConnectedPage:getChangeInfoText()
+	local patchData = self.props.patchData
+	if patchData == nil then
+		return ""
+	end
+
+	local elapsed = os.time() - patchData.timestamp
+	local changes = PatchSet.countChanges(patchData.patch)
+
+	return string.format("<i>Synced %d change%s %s</i>", changes, changes == 1 and "" or "s", timeSinceText(elapsed))
+end
+
 function ConnectedPage:init()
 	self.changeDrawerMotor = Flipper.SingleMotor.new(0)
 	self.changeDrawerHeight = bindingUtil.fromMotor(self.changeDrawerMotor)
@@ -176,6 +196,34 @@ function ConnectedPage:init()
 	self:setState({
 		renderChanges = 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)
+end
+
+function ConnectedPage:willUnmount()
+	if self.changeInfoTextUpdater then
+		task.cancel(self.changeInfoTextUpdater)
+	end
 end
 
 function ConnectedPage:render()
@@ -208,15 +256,7 @@ function ConnectedPage:render()
 			}),
 
 			ChangeInfo = e("TextButton", {
-				Text = self.props.patchInfo:map(function(info)
-					local changes = PatchSet.countChanges(info.patch)
-					return string.format(
-						"<i>Synced %d change%s %s</i>",
-						changes,
-						changes == 1 and "" or "s",
-						timeSinceText(os.time() - info.timestamp)
-					)
-				end),
+				Text = self.changeInfoText,
 				Font = Enum.Font.Gotham,
 				TextSize = 14,
 				TextWrapped = true,
@@ -249,7 +289,7 @@ function ConnectedPage:render()
 			ChangesDrawer = e(ChangesDrawer, {
 				rendered = self.state.renderChanges,
 				transparency = self.props.transparency,
-				patchInfo = self.props.patchInfo,
+				patch = self.props.patchData.patch,
 				serveSession = self.props.serveSession,
 				height = self.changeDrawerHeight,
 				layoutOrder = 4,
diff --git a/plugin/src/App/init.lua b/plugin/src/App/init.lua
index 35df8ffb..8c705449 100644
--- a/plugin/src/App/init.lua
+++ b/plugin/src/App/init.lua
@@ -51,10 +51,6 @@ function App:init()
 	self.host, self.setHost = Roact.createBinding(priorHost or "")
 	self.port, self.setPort = Roact.createBinding(priorPort or "")
 
-	self.patchInfo, self.setPatchInfo = Roact.createBinding({
-		patch = PatchSet.newEmpty(),
-		timestamp = os.time(),
-	})
 	self.confirmationBindable = Instance.new("BindableEvent")
 	self.confirmationEvent = self.confirmationBindable.Event
 
@@ -62,6 +58,10 @@ function App:init()
 		appStatus = AppStatus.NotConnected,
 		guiEnabled = false,
 		confirmData = {},
+		patchData = {
+			patch = PatchSet.newEmpty(),
+			timestamp = os.time(),
+		},
 		notifications = {},
 		toolbarIcon = Assets.Images.PluginButton,
 	})
@@ -227,21 +227,25 @@ function App:startSession()
 
 		local now = os.time()
 
-		local old = self.patchInfo:getValue()
+		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.setPatchInfo({
-				patch = merged,
-				timestamp = now,
+			self:setState({
+				patchData = {
+					patch = merged,
+					timestamp = now,
+				},
 			})
 		else
-			self.setPatchInfo({
-				patch = patch,
-				timestamp = now,
+			self:setState({
+				patchData = {
+					patch = patch,
+					timestamp = now,
+				},
 			})
 		end
 	end)
@@ -318,16 +322,6 @@ function App:startSession()
 	serveSession:start()
 
 	self.serveSession = serveSession
-
-	task.defer(function()
-		while self.serveSession == serveSession do
-			-- Trigger rerender to update timestamp text
-			local patchInfo = table.clone(self.patchInfo:getValue())
-			self.setPatchInfo(patchInfo)
-			local elapsed = os.time() - patchInfo.timestamp
-			task.wait(elapsed < 60 and 1 or elapsed / 5)
-		end
-	end)
 end
 
 function App:endSession()
@@ -429,7 +423,7 @@ function App:render()
 					Connected = createPageElement(AppStatus.Connected, {
 						projectName = self.state.projectName,
 						address = self.state.address,
-						patchInfo = self.patchInfo,
+						patchData = self.state.patchData,
 						serveSession = self.serveSession,
 
 						onDisconnect = function()

From a9ca77e27fea04bcdfd370e312955f2ec22a56d2 Mon Sep 17 00:00:00 2001
From: Boegie19 <34578426+Boegie19@users.noreply.github.com>
Date: Fri, 16 Jun 2023 20:41:30 +0200
Subject: [PATCH 03/16] Change gitmodules from ssh to https (#696)

Why if you want to use ssh you need todo more setup aka add a ssh key
into github while with https you don't have to do the extra work aka
makes rojo easier to contribute to.
---
 .gitmodules | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/.gitmodules b/.gitmodules
index 0842c711..2851e057 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -1,15 +1,15 @@
 [submodule "plugin/Packages/Roact"]
 	path = plugin/Packages/Roact
-	url = git@github.com:roblox/roact
+	url = https://github.com/roblox/roact.git
 [submodule "plugin/Packages/Flipper"]
 	path = plugin/Packages/Flipper
-	url = git@github.com:reselim/flipper
+	url = https://github.com/reselim/flipper.git
 [submodule "plugin/Packages/Promise"]
 	path = plugin/Packages/Promise
-	url = git@github.com:evaera/roblox-lua-promise
+	url = https://github.com/evaera/roblox-lua-promise.git
 [submodule "plugin/Packages/t"]
 	path = plugin/Packages/t
-	url = git@github.com:osyrisrblx/t
+	url = https://github.com/osyrisrblx/t.git
 [submodule "plugin/Packages/TestEZ"]
 	path = plugin/Packages/TestEZ
-	url = git@github.com:roblox/testez
+	url = https://github.com/roblox/testez.git

From 342fb57d143e335acc53e37b1855912b3ca537b1 Mon Sep 17 00:00:00 2001
From: Filip Tibell <filip.tibell@gmail.com>
Date: Fri, 30 Jun 2023 19:53:19 +0200
Subject: [PATCH 04/16] Fix compilation target in release workflow (#701)

Fixes macOS aarch64 builds not actually outputting x86 binaries by
specifying the `--target` flag during compilation in the release
workflow. These are the same changes as [this PR in
Aftman](https://github.com/LPGhatguy/aftman/pull/34), which had the same
issue, and uses the same workflow.
---
 .github/workflows/release.yml | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
index c4833c8d..5cc781b0 100644
--- a/.github/workflows/release.yml
+++ b/.github/workflows/release.yml
@@ -116,7 +116,7 @@ jobs:
           version: 'v0.2.6'
 
       - name: Build Release
-        run: cargo build --release --locked --verbose
+        run: cargo build --release --locked --verbose --target ${{ matrix.target }}
         env:
           # Build into a known directory so we can find our build artifact more
           # easily.
@@ -132,11 +132,11 @@ jobs:
           mkdir staging
 
           if [ "${{ matrix.host }}" = "windows" ]; then
-            cp "output/release/$BIN.exe" staging/
+            cp "output/${{ matrix.target }}/release/$BIN.exe" staging/
             cd staging
             7z a ../release.zip *
           else
-            cp "output/release/$BIN" staging/
+            cp "output/${{ matrix.target }}/release/$BIN" staging/
             cd staging
             zip ../release.zip *
           fi

From b303b0a99c29f53547d9e62603e89e7bfcab9dc8 Mon Sep 17 00:00:00 2001
From: boatbomber <zack@boatbomber.com>
Date: Fri, 30 Jun 2023 11:02:14 -0700
Subject: [PATCH 05/16] Add PRs 674 and 675 to changelog (#693)

Forgot to update changelog in those PRs, this remedies that
---
 CHANGELOG.md | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b0a7b948..68eb44b0 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,8 +2,12 @@
 
 ## Unreleased Changes
 * Significantly improved performance of `rojo sourcemap`. ([#668])
+* Fixed the diff visualizer of connected sessions. ([#674])
+* Fixed disconnected session activity. ([#675])
 
 [#668]: https://github.com/rojo-rbx/rojo/pull/668
+[#674]: https://github.com/rojo-rbx/rojo/pull/674
+[#675]: https://github.com/rojo-rbx/rojo/pull/675
 
 ## [7.3.0] - April 22, 2023
 * Added `$attributes` to project format. ([#574])

From 2507e096b78a5ccbe7df8913fbb416929b9cf329 Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Fri, 30 Jun 2023 11:03:09 -0700
Subject: [PATCH 06/16] Fix change processor clippy lints (#680)

Bear with me here...

The first in a series of PRs that aim to get CI passing
---
 src/change_processor.rs | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/change_processor.rs b/src/change_processor.rs
index f3902067..2429b590 100644
--- a/src/change_processor.rs
+++ b/src/change_processor.rs
@@ -251,7 +251,7 @@ fn on_vfs_event(
 
     let mut current_path = path.as_path();
     let affected_ids = loop {
-        let ids = tree.get_ids_at_path(&current_path);
+        let ids = tree.get_ids_at_path(current_path);
 
         log::trace!("Path {} affects IDs {:?}", current_path.display(), ids);
 
@@ -267,7 +267,7 @@ fn on_vfs_event(
     };
 
     for id in affected_ids {
-        if let Some(patch) = compute_and_apply_changes(&mut tree, &vfs, id) {
+        if let Some(patch) = compute_and_apply_changes(&mut tree, vfs, id) {
             if !patch.is_empty() {
                 applied_patches.push(patch);
             }
@@ -301,7 +301,7 @@ fn compute_and_apply_changes(tree: &mut RojoTree, vfs: &Vfs, id: Ref) -> Option<
                 // path still exists. We can generate a snapshot starting at
                 // that path and use it as the source for our patch.
 
-                let snapshot = match snapshot_from_vfs(&metadata.context, &vfs, &path) {
+                let snapshot = match snapshot_from_vfs(&metadata.context, vfs, path) {
                     Ok(snapshot) => snapshot,
                     Err(err) => {
                         log::error!("Snapshot error: {:?}", err);
@@ -309,7 +309,7 @@ fn compute_and_apply_changes(tree: &mut RojoTree, vfs: &Vfs, id: Ref) -> Option<
                     }
                 };
 
-                let patch_set = compute_patch_set(snapshot, &tree, id);
+                let patch_set = compute_patch_set(snapshot, tree, id);
                 apply_patch_set(tree, patch_set)
             }
             Ok(None) => {
@@ -337,10 +337,10 @@ fn compute_and_apply_changes(tree: &mut RojoTree, vfs: &Vfs, id: Ref) -> Option<
 
             let snapshot_result = snapshot_project_node(
                 &metadata.context,
-                &project_path,
+                project_path,
                 instance_name,
                 project_node,
-                &vfs,
+                vfs,
                 parent_class.as_ref().map(|name| name.as_str()),
             );
 
@@ -352,7 +352,7 @@ fn compute_and_apply_changes(tree: &mut RojoTree, vfs: &Vfs, id: Ref) -> Option<
                 }
             };
 
-            let patch_set = compute_patch_set(snapshot, &tree, id);
+            let patch_set = compute_patch_set(snapshot, tree, id);
             apply_patch_set(tree, patch_set)
         }
     };

From b7a1f82f569c0b3a22214ea40319833edd677932 Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Fri, 30 Jun 2023 11:03:37 -0700
Subject: [PATCH 07/16] Fix cli clippy lints (#681)

The second in a series of PRs that aim to get CI passing
---
 src/cli/fmt_project.rs | 2 +-
 src/cli/init.rs        | 2 +-
 src/cli/serve.rs       | 2 +-
 src/cli/upload.rs      | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/cli/fmt_project.rs b/src/cli/fmt_project.rs
index 48a52bd6..9c3e88d3 100644
--- a/src/cli/fmt_project.rs
+++ b/src/cli/fmt_project.rs
@@ -24,7 +24,7 @@ impl FmtProjectCommand {
         let serialized = serde_json::to_string_pretty(&project)
             .context("could not re-encode project file as JSON")?;
 
-        fs_err::write(&project.file_location, &serialized)
+        fs_err::write(&project.file_location, serialized)
             .context("could not write back to project file")?;
 
         Ok(())
diff --git a/src/cli/init.rs b/src/cli/init.rs
index 795ec04d..b770f55d 100644
--- a/src/cli/init.rs
+++ b/src/cli/init.rs
@@ -184,7 +184,7 @@ fn try_git_init(path: &Path, git_ignore: &str) -> Result<(), anyhow::Error> {
 /// already inside a Git repository.
 fn should_git_init(path: &Path) -> bool {
     let result = Command::new("git")
-        .args(&["rev-parse", "--is-inside-work-tree"])
+        .args(["rev-parse", "--is-inside-work-tree"])
         .stdout(Stdio::null())
         .stderr(Stdio::null())
         .current_dir(path)
diff --git a/src/cli/serve.rs b/src/cli/serve.rs
index 0e35785e..f89e4387 100644
--- a/src/cli/serve.rs
+++ b/src/cli/serve.rs
@@ -39,7 +39,7 @@ impl ServeCommand {
 
         let vfs = Vfs::new_default();
 
-        let session = Arc::new(ServeSession::new(vfs, &project_path)?);
+        let session = Arc::new(ServeSession::new(vfs, project_path)?);
 
         let ip = self
             .address
diff --git a/src/cli/upload.rs b/src/cli/upload.rs
index d8404099..ce7a2b5d 100644
--- a/src/cli/upload.rs
+++ b/src/cli/upload.rs
@@ -176,7 +176,7 @@ fn do_upload_open_cloud(
 
     log::debug!("Uploading to Roblox...");
     let response = client
-        .post(&url)
+        .post(url)
         .header("x-api-key", api_key)
         .header(CONTENT_TYPE, "application/xml")
         .header(ACCEPT, "application/json")

From 0dabd8a1f6361c4d76178d96097ead0bb5852e5e Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Fri, 30 Jun 2023 11:04:04 -0700
Subject: [PATCH 08/16] Fix memofs clippy lints (#682)

The third in a series of PRs that aim to get CI passing
---
 crates/memofs/src/in_memory_fs.rs | 6 ++++++
 crates/memofs/src/lib.rs          | 7 ++-----
 crates/memofs/src/noop_backend.rs | 6 ++++++
 crates/memofs/src/std_backend.rs  | 6 ++++++
 4 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/crates/memofs/src/in_memory_fs.rs b/crates/memofs/src/in_memory_fs.rs
index 328fbffe..fc8cf2ef 100644
--- a/crates/memofs/src/in_memory_fs.rs
+++ b/crates/memofs/src/in_memory_fs.rs
@@ -50,6 +50,12 @@ impl InMemoryFs {
     }
 }
 
+impl Default for InMemoryFs {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
 #[derive(Debug)]
 struct InMemoryFsInner {
     entries: HashMap<PathBuf, Entry>,
diff --git a/crates/memofs/src/lib.rs b/crates/memofs/src/lib.rs
index f6a17ef6..744ca60f 100644
--- a/crates/memofs/src/lib.rs
+++ b/crates/memofs/src/lib.rs
@@ -194,11 +194,8 @@ impl VfsInner {
     }
 
     fn commit_event(&mut self, event: &VfsEvent) -> io::Result<()> {
-        match event {
-            VfsEvent::Remove(path) => {
-                let _ = self.backend.unwatch(&path);
-            }
-            _ => {}
+        if let VfsEvent::Remove(path) = event {
+            let _ = self.backend.unwatch(path);
         }
 
         Ok(())
diff --git a/crates/memofs/src/noop_backend.rs b/crates/memofs/src/noop_backend.rs
index 383119c5..efc8fd4a 100644
--- a/crates/memofs/src/noop_backend.rs
+++ b/crates/memofs/src/noop_backend.rs
@@ -74,3 +74,9 @@ impl VfsBackend for NoopBackend {
         ))
     }
 }
+
+impl Default for NoopBackend {
+    fn default() -> Self {
+        Self::new()
+    }
+}
diff --git a/crates/memofs/src/std_backend.rs b/crates/memofs/src/std_backend.rs
index 7e616cbd..c13ade0b 100644
--- a/crates/memofs/src/std_backend.rs
+++ b/crates/memofs/src/std_backend.rs
@@ -108,3 +108,9 @@ impl VfsBackend for StdBackend {
             .map_err(|inner| io::Error::new(io::ErrorKind::Other, inner))
     }
 }
+
+impl Default for StdBackend {
+    fn default() -> Self {
+        Self::new()
+    }
+}

From e9efa238b01aaa9019ab0f33e139d8e20494eeb0 Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Fri, 30 Jun 2023 11:04:56 -0700
Subject: [PATCH 09/16] Fix project clippy lints (#683)

The fourth in a series of PRs that aim to get CI passing
---
 src/project.rs | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/src/project.rs b/src/project.rs
index 330cbd23..e70005d0 100644
--- a/src/project.rs
+++ b/src/project.rs
@@ -127,11 +127,10 @@ impl Project {
         contents: &[u8],
         project_file_location: &Path,
     ) -> Result<Self, ProjectError> {
-        let mut project: Self =
-            serde_json::from_slice(&contents).map_err(|source| Error::Json {
-                source,
-                path: project_file_location.to_owned(),
-            })?;
+        let mut project: Self = serde_json::from_slice(contents).map_err(|source| Error::Json {
+            source,
+            path: project_file_location.to_owned(),
+        })?;
 
         project.file_location = project_file_location.to_path_buf();
         project.check_compatibility();
@@ -197,8 +196,8 @@ pub enum PathNode {
 impl PathNode {
     pub fn path(&self) -> &Path {
         match self {
-            PathNode::Required(pathbuf) => &pathbuf,
-            PathNode::Optional(OptionalPathNode { optional }) => &optional,
+            PathNode::Required(pathbuf) => pathbuf,
+            PathNode::Optional(OptionalPathNode { optional }) => optional,
         }
     }
 }

From 5b1a090c5e520efe5b74265688391562c8354f14 Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Fri, 30 Jun 2023 11:05:30 -0700
Subject: [PATCH 10/16] Fix serve session clippy lints (#684)

The fifth in a series of PRs that aim to get CI passing
---
 src/serve_session.rs | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/src/serve_session.rs b/src/serve_session.rs
index 96a68ac9..9dab5446 100644
--- a/src/serve_session.rs
+++ b/src/serve_session.rs
@@ -102,12 +102,11 @@ impl ServeSession {
 
         log::trace!("Starting new ServeSession at path {}", start_path.display());
 
-        let project_path;
-        if Project::is_project_file(start_path) {
-            project_path = Cow::Borrowed(start_path);
+        let project_path = if Project::is_project_file(start_path) {
+            Cow::Borrowed(start_path)
         } else {
-            project_path = Cow::Owned(start_path.join("default.project.json"));
-        }
+            Cow::Owned(start_path.join("default.project.json"))
+        };
 
         log::debug!("Loading project file from {}", project_path.display());
 
@@ -127,7 +126,7 @@ impl ServeSession {
         let instance_context = InstanceContext::default();
 
         log::trace!("Generating snapshot of instances from VFS");
-        let snapshot = snapshot_from_vfs(&instance_context, &vfs, &start_path)?;
+        let snapshot = snapshot_from_vfs(&instance_context, &vfs, start_path)?;
 
         log::trace!("Computing initial patch set");
         let patch_set = compute_patch_set(snapshot, &tree, root_id);

From a2f68c2e3c670b3bc5c3dbf45ebbdd3d11ef3ccf Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Fri, 30 Jun 2023 11:05:55 -0700
Subject: [PATCH 11/16] Fix snapshot clippy lints (#685)

The sixth in a series of PRs that aim to get CI passing
---
 src/snapshot/patch_apply.rs   | 3 +--
 src/snapshot/patch_compute.rs | 6 ++----
 src/snapshot/tree.rs          | 8 ++++----
 3 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/src/snapshot/patch_apply.rs b/src/snapshot/patch_apply.rs
index 439452d8..5818776c 100644
--- a/src/snapshot/patch_apply.rs
+++ b/src/snapshot/patch_apply.rs
@@ -97,8 +97,7 @@ fn finalize_patch_application(context: PatchApplyContext, tree: &mut RojoTree) -
 
         for value in instance.properties_mut().values_mut() {
             if let Variant::Ref(referent) = value {
-                if let Some(&instance_referent) = context.snapshot_id_to_instance_id.get(&referent)
-                {
+                if let Some(&instance_referent) = context.snapshot_id_to_instance_id.get(referent) {
                     *value = Variant::Ref(instance_referent);
                 }
             }
diff --git a/src/snapshot/patch_compute.rs b/src/snapshot/patch_compute.rs
index 23e2f302..f5d5a67b 100644
--- a/src/snapshot/patch_compute.rs
+++ b/src/snapshot/patch_compute.rs
@@ -26,10 +26,8 @@ pub fn compute_patch_set(snapshot: Option<InstanceSnapshot>, tree: &RojoTree, id
         // for all of the IDs that we know about so far.
         rewrite_refs_in_updates(&context, &mut patch_set.updated_instances);
         rewrite_refs_in_additions(&context, &mut patch_set.added_instances);
-    } else {
-        if id != tree.get_root_id() {
-            patch_set.removed_instances.push(id);
-        }
+    } else if id != tree.get_root_id() {
+        patch_set.removed_instances.push(id);
     }
 
     patch_set
diff --git a/src/snapshot/tree.rs b/src/snapshot/tree.rs
index e342e186..e4379bc2 100644
--- a/src/snapshot/tree.rs
+++ b/src/snapshot/tree.rs
@@ -37,8 +37,8 @@ pub struct RojoTree {
 
 impl RojoTree {
     pub fn new(snapshot: InstanceSnapshot) -> RojoTree {
-        let root_builder = InstanceBuilder::new(snapshot.class_name.to_owned())
-            .with_name(snapshot.name.to_owned())
+        let root_builder = InstanceBuilder::new(snapshot.class_name)
+            .with_name(snapshot.name)
             .with_properties(snapshot.properties);
 
         let mut tree = RojoTree {
@@ -245,7 +245,7 @@ impl<'a> InstanceWithMeta<'a> {
     }
 
     pub fn metadata(&self) -> &'a InstanceMetadata {
-        &self.metadata
+        self.metadata
     }
 }
 
@@ -294,6 +294,6 @@ impl InstanceWithMetaMut<'_> {
     }
 
     pub fn metadata(&self) -> &InstanceMetadata {
-        &self.metadata
+        self.metadata
     }
 }

From 8646b2dfceb45d8a197cd9c4c3ed13444c6cadcc Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Fri, 30 Jun 2023 11:06:12 -0700
Subject: [PATCH 12/16] Fix snapshot middleware clippy lints (#686)

The seventh in a series of PRs that aim to get CI passing
---
 src/snapshot_middleware/dir.rs  | 2 +-
 src/snapshot_middleware/util.rs | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/snapshot_middleware/dir.rs b/src/snapshot_middleware/dir.rs
index ba63301c..092641e7 100644
--- a/src/snapshot_middleware/dir.rs
+++ b/src/snapshot_middleware/dir.rs
@@ -77,7 +77,7 @@ pub fn snapshot_dir_no_meta(
 
     let relevant_paths = vec![
         path.to_path_buf(),
-        meta_path.clone(),
+        meta_path,
         // TODO: We shouldn't need to know about Lua existing in this
         // middleware. Should we figure out a way for that function to add
         // relevant paths to this middleware?
diff --git a/src/snapshot_middleware/util.rs b/src/snapshot_middleware/util.rs
index 517cd606..16648eae 100644
--- a/src/snapshot_middleware/util.rs
+++ b/src/snapshot_middleware/util.rs
@@ -37,7 +37,7 @@ where
             .and_then(|name| name.to_str())
             .with_context(|| format!("Path did not have a file name: {}", path.display()))?;
 
-        match_trailing(&file_name, suffix)
+        match_trailing(file_name, suffix)
             .with_context(|| format!("Path did not end in {}: {}", suffix, path.display()))
     }
 }

From 67674d53a2d8337eebb56d16bd7b05efd6b6afcf Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Fri, 30 Jun 2023 11:06:43 -0700
Subject: [PATCH 13/16] Fix remaining clippy lints (#687)

The eighth (and final) in a series of PRs that aim to get CI passing
---
 build.rs               |  2 +-
 src/lua_ast.rs         | 30 ++++++++++++++++++++++++------
 src/path_serializer.rs |  2 +-
 src/web/interface.rs   |  6 +-----
 4 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/build.rs b/build.rs
index 1cff84c5..8326fcdc 100644
--- a/build.rs
+++ b/build.rs
@@ -54,7 +54,7 @@ fn main() -> Result<(), anyhow::Error> {
     });
 
     let out_path = Path::new(&out_dir).join("plugin.bincode");
-    let out_file = File::create(&out_path)?;
+    let out_file = File::create(out_path)?;
 
     bincode::serialize_into(out_file, &snapshot)?;
 
diff --git a/src/lua_ast.rs b/src/lua_ast.rs
index c31b609f..a61e41c9 100644
--- a/src/lua_ast.rs
+++ b/src/lua_ast.rs
@@ -219,12 +219,30 @@ fn is_valid_ident_char(value: char) -> bool {
 }
 
 fn is_keyword(value: &str) -> bool {
-    match value {
-        "and" | "break" | "do" | "else" | "elseif" | "end" | "false" | "for" | "function"
-        | "if" | "in" | "local" | "nil" | "not" | "or" | "repeat" | "return" | "then" | "true"
-        | "until" | "while" => true,
-        _ => false,
-    }
+    matches!(
+        value,
+        "and"
+            | "break"
+            | "do"
+            | "else"
+            | "elseif"
+            | "end"
+            | "false"
+            | "for"
+            | "function"
+            | "if"
+            | "in"
+            | "local"
+            | "nil"
+            | "not"
+            | "or"
+            | "repeat"
+            | "return"
+            | "then"
+            | "true"
+            | "until"
+            | "while"
+    )
 }
 
 /// Tells whether the given string is a valid Lua identifier.
diff --git a/src/path_serializer.rs b/src/path_serializer.rs
index 74eb9534..29edaf82 100644
--- a/src/path_serializer.rs
+++ b/src/path_serializer.rs
@@ -15,7 +15,7 @@ where
         .as_os_str()
         .to_str()
         .expect("Invalid Unicode in file path, cannot serialize");
-    let replaced = as_str.replace("\\", "/");
+    let replaced = as_str.replace('\\', "/");
 
     serializer.serialize_str(&replaced)
 }
diff --git a/src/web/interface.rs b/src/web/interface.rs
index 28005057..c10e14bc 100644
--- a/src/web/interface.rs
+++ b/src/web/interface.rs
@@ -145,11 +145,7 @@ fn property_filter(value: Option<&Variant>) -> bool {
 
     // Lua can't do anything with SharedString values. They also can't be
     // serialized directly by Serde!
-    if ty == Some(VariantType::SharedString) {
-        return false;
-    }
-
-    return true;
+    ty != Some(VariantType::SharedString)
 }
 
 /// Response body from /api/rojo

From 55ac231cecb7f350b8810cb8bcc57fc36a396a47 Mon Sep 17 00:00:00 2001
From: boatbomber <zack@boatbomber.com>
Date: Fri, 30 Jun 2023 11:21:36 -0700
Subject: [PATCH 14/16] Skip confirming patches that only contain a datamodel
 name change (#688)

Closes #672.

Skips the user confirmation if the patch contains only a datamodel name
change.

I decided to build generic PatchSet utility functions in case we need to
use similar logic in the future, in addition to maintaining clear
division of duties. The app code shouldn't be too dependent upon patch
internal structure when we can avoid it.

---------

Co-authored-by: Kenneth Loeffler <kenloef@gmail.com>
---
 CHANGELOG.md            |   2 +
 plugin/src/App/init.lua |  19 +++++++
 plugin/src/PatchSet.lua | 109 +++++++++++++++++++++++++++++++++++++++-
 3 files changed, 129 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 68eb44b0..07a20cf9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,10 +4,12 @@
 * Significantly improved performance of `rojo sourcemap`. ([#668])
 * Fixed the diff visualizer of connected sessions. ([#674])
 * Fixed disconnected session activity. ([#675])
+* Skip confirming patches that contain only a datamodel name change. ([#688])
 
 [#668]: https://github.com/rojo-rbx/rojo/pull/668
 [#674]: https://github.com/rojo-rbx/rojo/pull/674
 [#675]: https://github.com/rojo-rbx/rojo/pull/675
+[#688]: https://github.com/rojo-rbx/rojo/pull/688
 
 ## [7.3.0] - April 22, 2023
 * Added `$attributes` to project format. ([#574])
diff --git a/plugin/src/App/init.lua b/plugin/src/App/init.lua
index 8c705449..9d58b661 100644
--- a/plugin/src/App/init.lua
+++ b/plugin/src/App/init.lua
@@ -295,9 +295,28 @@ function App:startSession()
 
 	serveSession:setConfirmCallback(function(instanceMap, patch, serverInfo)
 		if PatchSet.isEmpty(patch) then
+			Log.trace("Accepting patch without confirmation because it is empty")
 			return "Accept"
 		end
 
+		-- The datamodel name gets overwritten by Studio, making confirmation of it intrusive
+		-- and unnecessary. This special case allows it to be accepted without confirmation.
+		if
+			PatchSet.hasAdditions(patch) == false
+			and PatchSet.hasRemoves(patch) == false
+			and PatchSet.containsOnlyInstance(patch, instanceMap, game)
+		then
+			local datamodelUpdates = PatchSet.getUpdateForInstance(patch, instanceMap, game)
+			if
+				datamodelUpdates ~= nil
+				and next(datamodelUpdates.changedProperties) == nil
+				and datamodelUpdates.changedClassName == nil
+			then
+				Log.trace("Accepting patch without confirmation because it only contains a datamodel name change")
+				return "Accept"
+			end
+		end
+
 		self:setState({
 			appStatus = AppStatus.Confirming,
 			confirmData = {
diff --git a/plugin/src/PatchSet.lua b/plugin/src/PatchSet.lua
index 3451de6a..c9f621dc 100644
--- a/plugin/src/PatchSet.lua
+++ b/plugin/src/PatchSet.lua
@@ -91,6 +91,113 @@ function PatchSet.hasUpdates(patchSet)
 	return next(patchSet.updated) ~= nil
 end
 
+--[[
+	Tells whether the given PatchSet contains changes to the given instance id
+]]
+function PatchSet.containsId(patchSet, instanceMap, id)
+	if patchSet.added[id] ~= nil then
+		return true
+	end
+
+	for _, idOrInstance in patchSet.removed do
+		local removedId = if Types.RbxId(idOrInstance) then idOrInstance else instanceMap.fromInstances[idOrInstance]
+		if removedId == id then
+			return true
+		end
+	end
+
+	for _, update in patchSet.updated do
+		if update.id == id then
+			return true
+		end
+	end
+
+	return false
+end
+
+--[[
+	Tells whether the given PatchSet contains changes to the given instance. 
+	If the given InstanceMap does not contain the instance, this function always returns false.
+]]
+function PatchSet.containsInstance(patchSet, instanceMap, instance)
+	local id = instanceMap.fromInstances[instance]
+	if id == nil then
+		return false
+	end
+
+	return PatchSet.containsId(patchSet, instanceMap, id)
+end
+
+--[[
+	Tells whether the given PatchSet contains changes to nothing but the given instance id
+]]
+function PatchSet.containsOnlyId(patchSet, instanceMap, id)
+	if not PatchSet.containsId(patchSet, instanceMap, id) then
+		-- Patch doesn't contain the id at all
+		return false
+	end
+
+	for addedId in patchSet.added do
+		if addedId ~= id then
+			return false
+		end
+	end
+
+	for _, idOrInstance in patchSet.removed do
+		local removedId = if Types.RbxId(idOrInstance) then idOrInstance else instanceMap.fromInstances[idOrInstance]
+		if removedId ~= id then
+			return false
+		end
+	end
+
+	for _, update in patchSet.updated do
+		if update.id ~= id then
+			return false
+		end
+	end
+
+	return true
+end
+
+--[[
+	Tells whether the given PatchSet contains changes to nothing but the given instance.
+	If the given InstanceMap does not contain the instance, this function always returns false.
+]]
+function PatchSet.containsOnlyInstance(patchSet, instanceMap, instance)
+	local id = instanceMap.fromInstances[instance]
+	if id == nil then
+		return false
+	end
+
+	return PatchSet.containsOnlyId(patchSet, instanceMap, id)
+end
+
+--[[
+	Returns the update to the given instance id, or nil if there aren't any
+]]
+function PatchSet.getUpdateForId(patchSet, id)
+	for _, update in patchSet.updated do
+		if update.id == id then
+			return update
+		end
+	end
+
+	return nil
+end
+
+--[[
+	Returns the update to the given instance, or nil if there aren't any.
+	If the given InstanceMap does not contain the instance, this function always returns nil.
+]]
+function PatchSet.getUpdateForInstance(patchSet, instanceMap, instance)
+	local id = instanceMap.fromInstances[instance]
+	if id == nil then
+		return nil
+	end
+
+	return PatchSet.getUpdateForId(patchSet, id)
+end
+
 --[[
 	Tells whether the given PatchSets are equal.
 ]]
@@ -150,7 +257,7 @@ function PatchSet.humanSummary(instanceMap, patchSet)
 	for _, idOrInstance in ipairs(patchSet.removed) do
 		local instance, id
 
-		if type(idOrInstance) == "string" then
+		if Types.RbxId(idOrInstance) then
 			id = idOrInstance
 			instance = instanceMap.fromIds[id]
 		else

From 66c1cd0d93d3560685d3e74c2de73d4d0f77ab38 Mon Sep 17 00:00:00 2001
From: boatbomber <zack@boatbomber.com>
Date: Fri, 30 Jun 2023 13:44:42 -0700
Subject: [PATCH 15/16] Add protection against syncing non place projects
 (#691)

Closes #113.

Does what it says on the tin. When you try to sync a non-place project,
it gives a clear message instead of crashing.
---
 CHANGELOG.md                |  2 ++
 plugin/src/ServeSession.lua | 16 ++++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 07a20cf9..33d39bdf 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,11 +5,13 @@
 * Fixed the diff visualizer of connected sessions. ([#674])
 * Fixed disconnected session activity. ([#675])
 * Skip confirming patches that contain only a datamodel name change. ([#688])
+* Added protection against syncing a model to a place. ([#691])
 
 [#668]: https://github.com/rojo-rbx/rojo/pull/668
 [#674]: https://github.com/rojo-rbx/rojo/pull/674
 [#675]: https://github.com/rojo-rbx/rojo/pull/675
 [#688]: https://github.com/rojo-rbx/rojo/pull/688
+[#691]: https://github.com/rojo-rbx/rojo/pull/691
 
 ## [7.3.0] - April 22, 2023
 * Added `$attributes` to project format. ([#574])
diff --git a/plugin/src/ServeSession.lua b/plugin/src/ServeSession.lua
index f77c8c74..08dc4041 100644
--- a/plugin/src/ServeSession.lua
+++ b/plugin/src/ServeSession.lua
@@ -232,6 +232,22 @@ function ServeSession:__initialSync(serverInfo)
 				Log.error("Could not compute a diff to catch up to the Rojo server: {:#?}", catchUpPatch)
 			end
 
+			for _, update in catchUpPatch.updated do
+				if
+					update.id == self.__instanceMap.fromInstances[game]
+					and update.changedClassName ~= nil
+				then
+					-- Non-place projects will try to update the classname of game from DataModel to
+					-- something like Folder, ModuleScript, etc. This would fail, so we exit with a clear
+					-- message instead of crashing.
+					return Promise.reject(
+						"Cannot sync a model as a place."
+						.. "\nEnsure Rojo is serving a project file that has a DataModel at the root of its tree and try again."
+						.. "\nSee project file docs: https://rojo.space/docs/v7/project-format/"
+					)
+				end
+			end
+
 			Log.trace("Computed hydration patch: {:#?}", debugPatch(catchUpPatch))
 
 			local userDecision = "Accept"

From 658d211779da4fb708ff28ea898efecdeb9e4d91 Mon Sep 17 00:00:00 2001
From: boatbomber <zack@boatbomber.com>
Date: Tue, 4 Jul 2023 13:09:41 -0700
Subject: [PATCH 16/16] Sync reminder notification & notification actions
 (#689)

Implements and closes #652.

---------

Co-authored-by: Chris Chang <51393127+chriscerie@users.noreply.github.com>
Co-authored-by: Micah <git@dekkonot.com>
---
 CHANGELOG.md                                  |   2 +
 plugin/src/App/Notifications.lua              | 109 ++++++++++++------
 .../src/App/StatusPages/Settings/Setting.lua  |   1 +
 plugin/src/App/StatusPages/Settings/init.lua  |  31 +++--
 plugin/src/App/init.lua                       |  97 ++++++++++++----
 plugin/src/Settings.lua                       |   1 +
 plugin/src/ignorePlaceIds.lua                 |  27 +++++
 7 files changed, 199 insertions(+), 69 deletions(-)
 create mode 100644 plugin/src/ignorePlaceIds.lua

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 33d39bdf..ec66d3c6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,12 +5,14 @@
 * Fixed the diff visualizer of connected sessions. ([#674])
 * Fixed disconnected session activity. ([#675])
 * 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])
 
 [#668]: https://github.com/rojo-rbx/rojo/pull/668
 [#674]: https://github.com/rojo-rbx/rojo/pull/674
 [#675]: https://github.com/rojo-rbx/rojo/pull/675
 [#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
 
 ## [7.3.0] - April 22, 2023
diff --git a/plugin/src/App/Notifications.lua b/plugin/src/App/Notifications.lua
index 5e1b0882..120a5aa5 100644
--- a/plugin/src/App/Notifications.lua
+++ b/plugin/src/App/Notifications.lua
@@ -7,6 +7,7 @@ local Packages = Rojo.Packages
 
 local Roact = require(Packages.Roact)
 local Flipper = require(Packages.Flipper)
+local Log = require(Packages.Log)
 
 local bindingUtil = require(script.Parent.bindingUtil)
 
@@ -14,6 +15,7 @@ local Theme = require(Plugin.App.Theme)
 local Assets = require(Plugin.Assets)
 
 local BorderedContainer = require(Plugin.App.Components.BorderedContainer)
+local TextButton = require(Plugin.App.Components.TextButton)
 
 local baseClock = DateTime.now().UnixTimestampMillis
 
@@ -28,10 +30,8 @@ function Notification:init()
 	self.lifetime = self.props.timeout
 
 	self.motor:onStep(function(value)
-		if value <= 0 then
-			if self.props.onClose then
-				self.props.onClose()
-			end
+		if value <= 0 and self.props.onClose then
+			self.props.onClose()
 		end
 	end)
 end
@@ -86,23 +86,54 @@ function Notification:willUnmount()
 end
 
 function Notification:render()
-	local time = DateTime.fromUnixTimestampMillis(self.props.timestamp)
-
-	local textBounds = TextService:GetTextSize(
-		self.props.text,
-		15,
-		Enum.Font.GothamSemibold,
-		Vector2.new(350, 700)
-	)
-
 	local transparency = self.binding:map(function(value)
 		return 1 - value
 	end)
 
+	local textBounds = TextService:GetTextSize(
+		self.props.text,
+		15,
+		Enum.Font.GothamMedium,
+		Vector2.new(350, 700)
+	)
+
+	local actionButtons = {}
+	local buttonsX = 0
+	if self.props.actions then
+		local count = 0
+		for key, action in self.props.actions do
+			actionButtons[key] = e(TextButton, {
+				text = action.text,
+				style = action.style,
+				onClick = function()
+					local success, err = pcall(action.onClick, self)
+					if not success then
+						Log.warn("Error in notification action: " .. tostring(err))
+					end
+				end,
+				layoutOrder = -action.layoutOrder,
+				transparency = transparency,
+			})
+
+			buttonsX += TextService:GetTextSize(
+				action.text, 18, Enum.Font.GothamMedium,
+				Vector2.new(math.huge, math.huge)
+			).X + 30
+
+			count += 1
+		end
+
+		buttonsX += (count - 1) * 5
+	end
+
+	local paddingY, logoSize = 20, 32
+	local actionsY = if self.props.actions then 35 else 0
+	local contentX = math.max(textBounds.X, buttonsX)
+
 	local size = self.binding:map(function(value)
 		return UDim2.fromOffset(
-			(35+40+textBounds.X)*value,
-			math.max(14+20+textBounds.Y, 32+20)
+			(35 + 40 + contentX) * value,
+			5 + actionsY + paddingY + math.max(logoSize, textBounds.Y)
 		)
 	end)
 
@@ -122,22 +153,22 @@ function Notification:render()
 				transparency = transparency,
 				size = UDim2.new(1, 0, 1, 0),
 			}, {
-				TextContainer = e("Frame", {
-					Size = UDim2.new(0, 35+textBounds.X, 1, -20),
-					Position = UDim2.new(0, 0, 0, 10),
+				Contents = e("Frame", {
+					Size = UDim2.new(0, 35 + contentX, 1, -paddingY),
+					Position = UDim2.new(0, 0, 0, paddingY / 2),
 					BackgroundTransparency = 1
 				}, {
 					Logo = e("ImageLabel", {
 						ImageTransparency = transparency,
 						Image = Assets.Images.PluginButton,
 						BackgroundTransparency = 1,
-						Size = UDim2.new(0, 32, 0, 32),
-						Position = UDim2.new(0, 0, 0.5, 0),
-						AnchorPoint = Vector2.new(0, 0.5),
+						Size = UDim2.new(0, logoSize, 0, logoSize),
+						Position = UDim2.new(0, 0, 0, 0),
+						AnchorPoint = Vector2.new(0, 0),
 					}),
 					Info = e("TextLabel", {
 						Text = self.props.text,
-						Font = Enum.Font.GothamSemibold,
+						Font = Enum.Font.GothamMedium,
 						TextSize = 15,
 						TextColor3 = theme.Notification.InfoColor,
 						TextTransparency = transparency,
@@ -150,20 +181,21 @@ function Notification:render()
 						LayoutOrder = 1,
 						BackgroundTransparency = 1,
 					}),
-					Time = e("TextLabel", {
-						Text = time:FormatLocalTime("LTS", "en-us"),
-						Font = Enum.Font.Code,
-						TextSize = 12,
-						TextColor3 = theme.Notification.InfoColor,
-						TextTransparency = transparency,
-						TextXAlignment = Enum.TextXAlignment.Left,
-
-						Size = UDim2.new(1, -35, 0, 14),
-						Position = UDim2.new(0, 35, 1, -14),
-
-						LayoutOrder = 1,
+					Actions = if self.props.actions then e("Frame", {
+						Size = UDim2.new(1, -40, 0, 35),
+						Position = UDim2.new(1, 0, 1, 0),
+						AnchorPoint = Vector2.new(1, 1),
 						BackgroundTransparency = 1,
-					}),
+					}, {
+						Layout = e("UIListLayout", {
+							FillDirection = Enum.FillDirection.Horizontal,
+							HorizontalAlignment = Enum.HorizontalAlignment.Right,
+							VerticalAlignment = Enum.VerticalAlignment.Center,
+							SortOrder = Enum.SortOrder.LayoutOrder,
+							Padding = UDim.new(0, 5),
+						}),
+						Buttons = Roact.createFragment(actionButtons),
+					}) else nil,
 				}),
 
 				Padding = e("UIPadding", {
@@ -180,15 +212,16 @@ local Notifications = Roact.Component:extend("Notifications")
 function Notifications:render()
 	local notifs = {}
 
-	for index, notif in ipairs(self.props.notifications) do
-		notifs[notif] = e(Notification, {
+	for id, notif in self.props.notifications do
+		notifs["NotifID_" .. id] = e(Notification, {
 			soundPlayer = self.props.soundPlayer,
 			text = notif.text,
 			timestamp = notif.timestamp,
 			timeout = notif.timeout,
+			actions = notif.actions,
 			layoutOrder = (notif.timestamp - baseClock),
 			onClose = function()
-				self.props.onClose(index)
+				self.props.onClose(id)
 			end,
 		})
 	end
diff --git a/plugin/src/App/StatusPages/Settings/Setting.lua b/plugin/src/App/StatusPages/Settings/Setting.lua
index 8bf688f6..0c0de9f4 100644
--- a/plugin/src/App/StatusPages/Settings/Setting.lua
+++ b/plugin/src/App/StatusPages/Settings/Setting.lua
@@ -59,6 +59,7 @@ function Setting:render()
 			LayoutOrder = self.props.layoutOrder,
 			ZIndex = -self.props.layoutOrder,
 			BackgroundTransparency = 1,
+			Visible = self.props.visible,
 
 			[Roact.Change.AbsoluteSize] = function(object)
 				self.setContainerSize(object.AbsoluteSize)
diff --git a/plugin/src/App/StatusPages/Settings/init.lua b/plugin/src/App/StatusPages/Settings/init.lua
index 1d484423..f83e145c 100644
--- a/plugin/src/App/StatusPages/Settings/init.lua
+++ b/plugin/src/App/StatusPages/Settings/init.lua
@@ -87,19 +87,20 @@ function SettingsPage:render()
 				layoutOrder = 0,
 			}),
 
-			OpenScriptsExternally = e(Setting, {
-				id = "openScriptsExternally",
-				name = "Open Scripts Externally",
-				description = "Attempt to open scripts in an external editor",
-				transparency = self.props.transparency,
-				layoutOrder = 1,
-			}),
-
 			ShowNotifications = e(Setting, {
 				id = "showNotifications",
 				name = "Show Notifications",
 				description = "Popup notifications in viewport",
 				transparency = self.props.transparency,
+				layoutOrder = 1,
+			}),
+
+			SyncReminder = e(Setting, {
+				id = "syncReminder",
+				name = "Sync Reminder",
+				description = "Notify to sync when opening a place that has previously been synced",
+				transparency = self.props.transparency,
+				visible = Settings:getBinding("showNotifications"),
 				layoutOrder = 2,
 			}),
 
@@ -111,12 +112,20 @@ function SettingsPage:render()
 				layoutOrder = 3,
 			}),
 
+			OpenScriptsExternally = e(Setting, {
+				id = "openScriptsExternally",
+				name = "Open Scripts Externally",
+				description = "EXPERIMENTAL! Attempt to open scripts in an external editor",
+				transparency = self.props.transparency,
+				layoutOrder = 4,
+			}),
+
 			TwoWaySync = e(Setting, {
 				id = "twoWaySync",
 				name = "Two-Way Sync",
 				description = "EXPERIMENTAL! Editing files in Studio will sync them into the filesystem",
 				transparency = self.props.transparency,
-				layoutOrder = 4,
+				layoutOrder = 5,
 			}),
 
 			LogLevel = e(Setting, {
@@ -124,7 +133,7 @@ function SettingsPage:render()
 				name = "Log Level",
 				description = "Plugin output verbosity level",
 				transparency = self.props.transparency,
-				layoutOrder = 5,
+				layoutOrder = 100,
 
 				options = invertedLevels,
 				showReset = Settings:getBinding("logLevel"):map(function(value)
@@ -140,7 +149,7 @@ function SettingsPage:render()
 				name = "Typechecking",
 				description = "Toggle typechecking on the API surface",
 				transparency = self.props.transparency,
-				layoutOrder = 6,
+				layoutOrder = 101,
 			}),
 
 			Layout = e("UIListLayout", {
diff --git a/plugin/src/App/init.lua b/plugin/src/App/init.lua
index 9d58b661..9fe081d7 100644
--- a/plugin/src/App/init.lua
+++ b/plugin/src/App/init.lua
@@ -1,5 +1,6 @@
 local Players = game:GetService("Players")
 local ServerStorage = game:GetService("ServerStorage")
+local RunService = game:GetService("RunService")
 
 local Rojo = script:FindFirstAncestor("Rojo")
 local Plugin = Rojo.Plugin
@@ -19,6 +20,7 @@ local ApiContext = require(Plugin.ApiContext)
 local PatchSet = require(Plugin.PatchSet)
 local preloadAssets = require(Plugin.preloadAssets)
 local soundPlayer = require(Plugin.soundPlayer)
+local ignorePlaceIds = require(Plugin.ignorePlaceIds)
 local Theme = require(script.Theme)
 
 local Page = require(script.Page)
@@ -53,6 +55,7 @@ function App:init()
 
 	self.confirmationBindable = Instance.new("BindableEvent")
 	self.confirmationEvent = self.confirmationBindable.Event
+	self.notifId = 0
 
 	self:setState({
 		appStatus = AppStatus.NotConnected,
@@ -65,28 +68,63 @@ function App:init()
 		notifications = {},
 		toolbarIcon = Assets.Images.PluginButton,
 	})
+
+	if
+		RunService:IsEdit()
+		and self.serveSession == nil
+		and Settings:get("syncReminder")
+		and self:getLastSyncTimestamp()
+	then
+		self:addNotification("You've previously synced this place. Would you like to reconnect?", 300, {
+			Connect = {
+				text = "Connect",
+				style = "Solid",
+				layoutOrder = 1,
+				onClick = function(notification)
+					notification:dismiss()
+					self:startSession()
+				end
+			},
+			Dismiss = {
+				text = "Dismiss",
+				style = "Bordered",
+				layoutOrder = 2,
+				onClick = function(notification)
+					notification:dismiss()
+				end,
+			},
+		})
+	end
 end
 
-function App:addNotification(text: string, timeout: number?)
+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
 
+	self.notifId += 1
+	local id = self.notifId
+
 	local notifications = table.clone(self.state.notifications)
-	table.insert(notifications, {
+	notifications[id] = {
 		text = text,
 		timestamp = DateTime.now().UnixTimestampMillis,
 		timeout = timeout or 3,
-	})
+		actions = actions,
+	}
 
 	self:setState({
 		notifications = notifications,
 	})
+
+	return function()
+		self:closeNotification(id)
+	end
 end
 
-function App:closeNotification(index: number)
+function App:closeNotification(id: number)
 	local notifications = table.clone(self.state.notifications)
-	table.remove(notifications, index)
+	notifications[id] = nil
 
 	self:setState({
 		notifications = notifications,
@@ -97,12 +135,28 @@ function App:getPriorEndpoint()
 	local priorEndpoints = Settings:get("priorEndpoints")
 	if not priorEndpoints then return end
 
-	local place = priorEndpoints[tostring(game.PlaceId)]
+	local id = tostring(game.PlaceId)
+	if ignorePlaceIds[id] then return end
+
+	local place = priorEndpoints[id]
 	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
+
+	local id = tostring(game.PlaceId)
+	if ignorePlaceIds[id] then return end
+
+	local place = priorEndpoints[id]
+	if not place then return end
+
+	return place.timestamp
+end
+
 function App:setPriorEndpoint(host: string, port: string)
 	local priorEndpoints = Settings:get("priorEndpoints")
 	if not priorEndpoints then
@@ -117,17 +171,16 @@ function App:setPriorEndpoint(host: string, port: string)
 		end
 	end
 
-	if host == Config.defaultHost and port == Config.defaultPort then
-		-- Don't save default
-		priorEndpoints[tostring(game.PlaceId)] = nil
-	else
-		priorEndpoints[tostring(game.PlaceId)] = {
-			host = host ~= Config.defaultHost and host or nil,
-			port = port ~= Config.defaultPort and port or nil,
-			timestamp = os.time(),
-		}
-		Log.trace("Saved last used endpoint for {}", game.PlaceId)
-	end
+	local id = tostring(game.PlaceId)
+	if ignorePlaceIds[id] then return end
+
+	priorEndpoints[id] = {
+		host = if host ~= Config.defaultHost then host else nil,
+		port = if port ~= Config.defaultPort then port else nil,
+		timestamp = os.time(),
+	}
+	Log.trace("Saved last used endpoint for {}", game.PlaceId)
+
 
 	Settings:set("priorEndpoints", priorEndpoints)
 end
@@ -470,7 +523,11 @@ function App:render()
 					}),
 				}),
 
-				RojoNotifications = e("ScreenGui", {}, {
+				RojoNotifications = e("ScreenGui", {
+					ZIndexBehavior = Enum.ZIndexBehavior.Sibling,
+					ResetOnSpawn = false,
+					DisplayOrder = 100,
+				}, {
 					layout = e("UIListLayout", {
 						SortOrder = Enum.SortOrder.LayoutOrder,
 						HorizontalAlignment = Enum.HorizontalAlignment.Right,
@@ -486,8 +543,8 @@ function App:render()
 					notifs = e(Notifications, {
 						soundPlayer = self.props.soundPlayer,
 						notifications = self.state.notifications,
-						onClose = function(index)
-							self:closeNotification(index)
+						onClose = function(id)
+							self:closeNotification(id)
 						end,
 					}),
 				}),
diff --git a/plugin/src/Settings.lua b/plugin/src/Settings.lua
index 0f23c86b..f6368a4d 100644
--- a/plugin/src/Settings.lua
+++ b/plugin/src/Settings.lua
@@ -13,6 +13,7 @@ local defaultSettings = {
 	openScriptsExternally = false,
 	twoWaySync = false,
 	showNotifications = true,
+	syncReminder = true,
 	playSounds = true,
 	typecheckingEnabled = false,
 	logLevel = "Info",
diff --git a/plugin/src/ignorePlaceIds.lua b/plugin/src/ignorePlaceIds.lua
new file mode 100644
index 00000000..40ad8f58
--- /dev/null
+++ b/plugin/src/ignorePlaceIds.lua
@@ -0,0 +1,27 @@
+--[[
+    These are place ids that will not have metadata saved for them,
+    such as last sync address or time. This is because they are not unique
+    so storing metadata for them does not make sense as these ids are reused.
+--]]
+
+return {
+    ["0"] = true, -- Local file
+    ["95206881"] = true, -- Baseplate
+    ["6560363541"] = true, -- Classic Baseplate
+    ["95206192"] = true, -- Flat Terrain
+    ["13165709401"] = true, -- Modern City
+    ["520390648"] = true, -- Village
+    ["203810088"] = true, -- Castle
+    ["366130569"] = true, -- Suburban
+    ["215383192"] = true, -- Racing
+    ["264719325"] = true, -- Pirate Island
+    ["203812057"] = true, -- Obby
+    ["379736082"] = true, -- Starting Place
+    ["301530843"] = true, -- Line Runner
+    ["92721754"] = true, -- Capture The Flag
+    ["301529772"] = true, -- Team/FFA Arena
+    ["203885589"] = true, -- Combat
+    ["10275826693"] = true, -- Concert
+    ["5353920686"] = true, -- Move It Simulator
+    ["6936227200"] = true, -- Mansion Of Wonder
+}