Merge branch 'master' of https://github.com/rojo-rbx/rojo into headless-api

plugin/src/App/StatusPages/Settings/init.lua

@@ -76,6 +76,12 @@ function SettingsPage:init()
 end
 
 function SettingsPage:render()
+	local layoutOrder = 0
+	local function layoutIncrement()
+		layoutOrder += 1
+		return layoutOrder
+	end
+
 	return Theme.with(function(theme)
 		theme = theme.Settings
 
@@ -87,7 +93,7 @@ function SettingsPage:render()
 			Navbar = e(Navbar, {
 				onBack = self.props.onBack,
 				transparency = self.props.transparency,
-				layoutOrder = 0,
+				layoutOrder = layoutIncrement(),
 			}),
 
 			ShowNotifications = e(Setting, {
@@ -95,7 +101,7 @@ function SettingsPage:render()
 				name = "Show Notifications",
 				description = "Popup notifications in viewport",
 				transparency = self.props.transparency,
-				layoutOrder = 1,
+				layoutOrder = layoutIncrement(),
 			}),
 
 			SyncReminder = e(Setting, {
@@ -104,7 +110,7 @@ function SettingsPage:render()
 				description = "Notify to sync when opening a place that has previously been synced",
 				transparency = self.props.transparency,
 				visible = Settings:getBinding("showNotifications"),
-				layoutOrder = 2,
+				layoutOrder = layoutIncrement(),
 			}),
 
 			ConfirmationBehavior = e(Setting, {
@@ -112,7 +118,7 @@ function SettingsPage:render()
 				name = "Confirmation Behavior",
 				description = "When to prompt for confirmation before syncing",
 				transparency = self.props.transparency,
-				layoutOrder = 3,
+				layoutOrder = layoutIncrement(),
 
 				options = confirmationBehaviors,
 			}),
@@ -122,7 +128,7 @@ function SettingsPage:render()
 				name = "Confirmation Threshold",
 				description = "How many modified instances to be considered a large change",
 				transparency = self.props.transparency,
-				layoutOrder = 4,
+				layoutOrder = layoutIncrement(),
 				visible = Settings:getBinding("confirmationBehavior"):map(function(value)
 					return value == "Large Changes"
 				end),
@@ -153,7 +159,16 @@ function SettingsPage:render()
 				name = "Play Sounds",
 				description = "Toggle sound effects",
 				transparency = self.props.transparency,
-				layoutOrder = 5,
+				layoutOrder = layoutIncrement(),
+			}),
+
+			AutoConnectPlaytestServer = e(Setting, {
+				id = "autoConnectPlaytestServer",
+				name = "Auto Connect Playtest Server",
+				description = "Automatically connect game server to Rojo when playtesting while connected in Edit",
+				experimental = true,
+				transparency = self.props.transparency,
+				layoutOrder = layoutIncrement(),
 			}),
 
 			Permissions = e(Setting, {
@@ -203,7 +218,7 @@ function SettingsPage:render()
 				locked = self.props.syncActive,
 				experimental = true,
 				transparency = self.props.transparency,
-				layoutOrder = 7,
+				layoutOrder = layoutIncrement(),
 			}),
 
 			TwoWaySync = e(Setting, {
@@ -213,7 +228,7 @@ function SettingsPage:render()
 				locked = self.props.syncActive,
 				experimental = true,
 				transparency = self.props.transparency,
-				layoutOrder = 8,
+				layoutOrder = layoutIncrement(),
 			}),
 
 			LogLevel = e(Setting, {
@@ -221,7 +236,7 @@ function SettingsPage:render()
 				name = "Log Level",
 				description = "Plugin output verbosity level",
 				transparency = self.props.transparency,
-				layoutOrder = 100,
+				layoutOrder = layoutIncrement(),
 
 				options = invertedLevels,
 				showReset = Settings:getBinding("logLevel"):map(function(value)
@@ -237,7 +252,7 @@ function SettingsPage:render()
 				name = "Typechecking",
 				description = "Toggle typechecking on the API surface",
 				transparency = self.props.transparency,
-				layoutOrder = 101,
+				layoutOrder = layoutIncrement(),
 			}),
 
 			Layout = e("UIListLayout", {

plugin/src/App/init.lua

@@ -214,11 +214,29 @@ function App:init()
 			},
 		})
 	end
+
+	if self:isAutoConnectPlaytestServerAvailable() then
+		self:useRunningConnectionInfo()
+		self:startSession()
+	end
+	self.autoConnectPlaytestServerListener = Settings:onChanged("autoConnectPlaytestServer", function(enabled)
+		if enabled then
+			if self:isAutoConnectPlaytestServerWriteable() and self.serveSession ~= nil then
+				-- Write the existing session
+				local baseUrl = self.serveSession.__apiContext.__baseUrl
+				self:setRunningConnectionInfo(baseUrl)
+			end
+		else
+			self:clearRunningConnectionInfo()
+		end
+	end)
 end
 
 function App:willUnmount()
 	self.waypointConnection:Disconnect()
 	self.confirmationBindable:Destroy()
+	self.autoConnectPlaytestServerListener()
+	self:clearRunningConnectionInfo()
 end
 
 function App:addNotification(
@@ -368,10 +386,7 @@ function App:getHostAndPort(): (string, string)
 	local host = self.host:getValue()
 	local port = self.port:getValue()
 
-	local host = if #host > 0 then host else Config.defaultHost
-	local port = if #port > 0 then port else Config.defaultPort
-
-	return host, port
+	return if #host > 0 then host else Config.defaultHost, if #port > 0 then port else Config.defaultPort
 end
 
 function App:isSyncLockAvailable()
@@ -493,6 +508,49 @@ function App:requestPermission(
 	return response
 end
 
+function App:isAutoConnectPlaytestServerAvailable()
+	return RunService:IsRunMode()
+		and RunService:IsServer()
+		and Settings:get("autoConnectPlaytestServer")
+		and workspace:GetAttribute("__Rojo_ConnectionUrl")
+end
+
+function App:isAutoConnectPlaytestServerWriteable()
+	return RunService:IsEdit() and Settings:get("autoConnectPlaytestServer")
+end
+
+function App:setRunningConnectionInfo(baseUrl: string)
+	if not self:isAutoConnectPlaytestServerWriteable() then
+		return
+	end
+
+	Log.trace("Setting connection info for play solo auto-connect")
+	workspace:SetAttribute("__Rojo_ConnectionUrl", baseUrl)
+end
+
+function App:clearRunningConnectionInfo()
+	if not RunService:IsEdit() then
+		-- Only write connection info from edit mode
+		return
+	end
+
+	Log.trace("Clearing connection info for play solo auto-connect")
+	workspace:SetAttribute("__Rojo_ConnectionUrl", nil)
+end
+
+function App:useRunningConnectionInfo()
+	local connectionInfo = workspace:GetAttribute("__Rojo_ConnectionUrl")
+	if not connectionInfo then
+		return
+	end
+
+	Log.trace("Using connection info for play solo auto-connect")
+	local host, port = string.match(connectionInfo, "^(.+):(.-)$")
+
+	self.setHost(host)
+	self.setPort(port)
+end
+
 function App:startSession(host: string?, port: string?)
 	local claimedLock, priorOwner = self:claimSyncLock()
 	if not claimedLock then
@@ -592,6 +650,7 @@ function App:startSession(host: string?, port: string?)
 			self.headlessAPI:_updateProperty("ProjectName", details)
 
 			self.knownProjects[details] = true
+			self:setRunningConnectionInfo(baseUrl)
 
 			self:setState({
 				appStatus = AppStatus.Connected,
@@ -603,6 +662,7 @@ function App:startSession(host: string?, port: string?)
 		elseif status == ServeSession.Status.Disconnected then
 			self.serveSession = nil
 			self:releaseSyncLock()
+			self:clearRunningConnectionInfo()
 			self:setState({
 				patchData = {
 					patch = PatchSet.newEmpty(),
@@ -644,6 +704,12 @@ function App:startSession(host: string?, port: string?)
 			return "Accept"
 		end
 
+		-- Play solo auto-connect does not require confirmation
+		if self:isAutoConnectPlaytestServerAvailable() then
+			Log.trace("Accepting patch without confirmation because play solo auto-connect is enabled")
+			return "Accept"
+		end
+
 		local confirmationBehavior = Settings:get("confirmationBehavior")
 		if confirmationBehavior == "Initial" then
 			-- Only confirm if we haven't synced this project yet this session
@@ -779,7 +845,7 @@ function App:render()
 		value = self.props.plugin,
 	}, {
 		e(Theme.StudioProvider, nil, {
-			e(Tooltip.Provider, nil, {
+			tooltip = e(Tooltip.Provider, nil, {
 				popups = Roact.createFragment(popups),
 
 				gui = e(StudioPluginGui, {

plugin/src/InstanceMap.lua

@@ -113,27 +113,29 @@ end
 function InstanceMap:destroyInstance(instance)
 	local id = self.fromInstances[instance]
 
+	local descendants = instance:GetDescendants()
+	instance:Destroy()
+
+	-- After the instance is successfully destroyed,
+	-- we can remove all the id mappings
+
 	if id ~= nil then
 		self:removeId(id)
 	end
 
-	for _, descendantInstance in ipairs(instance:GetDescendants()) do
+	for _, descendantInstance in descendants do
 		self:removeInstance(descendantInstance)
 	end
-
-	instance:Destroy()
 end
 
 function InstanceMap:destroyId(id)
 	local instance = self.fromIds[id]
-	self:removeId(id)
-
 	if instance ~= nil then
-		for _, descendantInstance in ipairs(instance:GetDescendants()) do
-			self:removeInstance(descendantInstance)
-		end
-
-		instance:Destroy()
+		self:destroyInstance(instance)
+	else
+		-- There is no instance with this id, so we can just remove the id
+		-- without worrying about instance destruction
+		self:removeId(id)
 	end
 end
 

plugin/src/PatchTree.lua

@@ -426,22 +426,71 @@ function PatchTree.updateMetadata(tree, patch, instanceMap, unappliedPatch)
 	-- 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
+		if not node then
+			continue
 		end
+
+		node.isWarning = true
+		Log.trace("Marked node as warning: {} {}", node.id, node.name)
+
+		if not node.changeList then
+			continue
+		end
+		for _, change in node.changeList do
+			local property = change[1]
+			local propertyFailedToApply = if property == "Name"
+				then failedChange.changedName ~= nil -- Name is not in changedProperties, so it needs a special case
+				else failedChange.changedProperties[property] ~= nil
+
+			if not propertyFailedToApply then
+				-- This change didn't fail, no need to mark
+				continue
+			end
+			if change[4] == nil then
+				change[4] = { isWarning = true }
+			else
+				change[4].isWarning = true
+			end
+			Log.trace("  Marked property as warning: {}.{}", node.name, property)
+		end
+	end
+	for failedAdditionId in unappliedPatch.added do
+		local node = tree:getNode(failedAdditionId)
+		if not node then
+			continue
+		end
+
+		node.isWarning = true
+		Log.trace("Marked node as warning: {} {}", node.id, node.name)
+
+		if not node.changeList then
+			continue
+		end
+		for _, change in node.changeList do
+			-- Failed addition means that all properties failed to be added
+			if change[4] == nil then
+				change[4] = { isWarning = true }
+			else
+				change[4].isWarning = true
+			end
+			Log.trace("  Marked property as warning: {}.{}", node.name, change[1])
+		end
+	end
+	for _, failedRemovalIdOrInstance in unappliedPatch.removed do
+		local failedRemovalId = if Types.RbxId(failedRemovalIdOrInstance)
+			then failedRemovalIdOrInstance
+			else instanceMap.fromInstances[failedRemovalIdOrInstance]
+		if not failedRemovalId then
+			continue
+		end
+
+		local node = tree:getNode(failedRemovalId)
+		if not node then
+			continue
+		end
+
+		node.isWarning = true
+		Log.trace("Marked node as warning: {} {}", node.id, node.name)
 	end
 
 	-- Update if instances exist

plugin/src/Reconciler/applyPatch.lua

@@ -25,10 +25,15 @@ local function applyPatch(instanceMap, patch)
 	local unappliedPatch = PatchSet.newEmpty()
 
 	for _, removedIdOrInstance in ipairs(patch.removed) do
-		if Types.RbxId(removedIdOrInstance) then
-			instanceMap:destroyId(removedIdOrInstance)
-		else
-			instanceMap:destroyInstance(removedIdOrInstance)
+		local ok = pcall(function()
+			if Types.RbxId(removedIdOrInstance) then
+				instanceMap:destroyId(removedIdOrInstance)
+			else
+				instanceMap:destroyInstance(removedIdOrInstance)
+			end
+		end)
+		if not ok then
+			table.insert(unappliedPatch.removed, removedIdOrInstance)
 		end
 	end
 
@@ -170,7 +175,13 @@ local function applyPatch(instanceMap, patch)
 		end
 
 		if update.changedName ~= nil then
-			instance.Name = update.changedName
+			local ok = pcall(function()
+				instance.Name = update.changedName
+			end)
+			if not ok then
+				unappliedUpdate.changedName = update.changedName
+				partiallyApplied = true
+			end
 		end
 
 		if update.changedMetadata ~= nil then

plugin/src/Settings.lua

@@ -14,6 +14,7 @@ local defaultSettings = {
 	twoWaySync = false,
 	showNotifications = true,
 	syncReminder = true,
+	autoConnectPlaytestServer = false,
 	confirmationBehavior = "Initial",
 	largeChangesConfirmationThreshold = 5,
 	playSounds = true,