Start rewriting plugin on top of new sync protocol

plugin/src/ApiContext.lua

@@ -1,152 +1,133 @@
-local Promise = require(script.Parent.Parent.Promise)
 local Http = require(script.Parent.Parent.Http)
+local Promise = require(script.Parent.Parent.Promise)
 
 local Config = require(script.Parent.Config)
-local Version = require(script.Parent.Version)
 local Types = require(script.Parent.Types)
+local Version = require(script.Parent.Version)
 
 local validateApiInfo = Types.ifEnabled(Types.ApiInfoResponse)
 local validateApiRead = Types.ifEnabled(Types.ApiReadResponse)
 local validateApiSubscribe = Types.ifEnabled(Types.ApiSubscribeResponse)
 
-local ApiContext = {}
-ApiContext.__index = ApiContext
-
--- TODO: Audit cases of errors and create enum values for each of them.
-ApiContext.Error = {
-	ServerIdMismatch = "ServerIdMismatch",
-
-	-- The server gave an unexpected 400-category error, which may be the
-	-- client's fault.
-	ClientError = "ClientError",
-
-	-- The server gave an unexpected 500-category error, which may be the
-	-- server's fault.
-	ServerError = "ServerError",
-}
-
-setmetatable(ApiContext.Error, {
-	__index = function(_, key)
-		error("Invalid ApiContext.Error name " .. key, 2)
-	end
-})
-
 local function rejectFailedRequests(response)
 	if response.code >= 400 then
-		if response.code < 500 then
+		-- TODO: Nicer error types for responses, using response JSON if valid.
-			return Promise.reject(ApiContext.Error.ClientError)
+		return Promise.reject(tostring(response.code))
-		else
-			return Promise.reject(ApiContext.Error.ServerError)
-		end
 	end
 
 	return response
 end
 
+local function rejectWrongProtocolVersion(infoResponseBody)
+	if infoResponseBody.protocolVersion ~= Config.protocolVersion then
+		local message = (
+			"Found a Rojo dev server, but it's using a different protocol version, and is incompatible." ..
+			"\nMake sure you have matching versions of both the Rojo plugin and server!" ..
+			"\n\nYour client is version %s, with protocol version %s. It expects server version %s." ..
+			"\nYour server is version %s, with protocol version %s." ..
+			"\n\nGo to https://github.com/rojo-rbx/rojo for more details."
+		):format(
+			Version.display(Config.version), Config.protocolVersion,
+			Config.expectedServerVersionString,
+			infoResponseBody.serverVersion, infoResponseBody.protocolVersion
+		)
+
+		return Promise.reject(message)
+	end
+
+	return Promise.resolve(infoResponseBody)
+end
+
+local function rejectWrongPlaceId(infoResponseBody)
+	if infoResponseBody.expectedPlaceIds ~= nil then
+		local foundId = false
+
+		for _, id in ipairs(infoResponseBody.expectedPlaceIds) do
+			if id == game.PlaceId then
+				foundId = true
+				break
+			end
+		end
+
+		if not foundId then
+			local idList = {}
+			for _, id in ipairs(infoResponseBody.expectedPlaceIds) do
+				table.insert(idList, "- " .. tostring(id))
+			end
+
+			local message = (
+				"Found a Rojo server, but its project is set to only be used with a specific list of places." ..
+				"\nYour place ID is %s, but needs to be one of these:" ..
+				"\n%s" ..
+				"\n\nTo change this list, edit 'servePlaceIds' in your .project.json file."
+			):format(
+				tostring(game.PlaceId),
+				table.concat(idList, "\n")
+			)
+
+			return Promise.reject(message)
+		end
+	end
+
+	return Promise.resolve(infoResponseBody)
+end
+
+local ApiContext = {}
+ApiContext.__index = ApiContext
+
 function ApiContext.new(baseUrl)
 	assert(type(baseUrl) == "string")
 
 	local self = {
-		baseUrl = baseUrl,
+		__baseUrl = baseUrl,
-		serverId = nil,
+		__serverId = nil,
-		rootInstanceId = nil,
+		__messageCursor = -1,
-		messageCursor = -1,
-		partitionRoutes = nil,
 	}
 
-	setmetatable(self, ApiContext)
+	return setmetatable(self, ApiContext)
-
-	return self
-end
-
-function ApiContext:onMessage(callback)
-	self.onMessageCallback = callback
 end
 
 function ApiContext:connect()
-	local url = ("%s/api/rojo"):format(self.baseUrl)
+	local url = ("%s/api/rojo"):format(self.__baseUrl)
 
 	return Http.get(url)
 		:andThen(rejectFailedRequests)
-		:andThen(function(response)
+		:andThen(Http.Response.json)
-			local body = response:json()
+		:andThen(rejectWrongProtocolVersion)
-
+		:andThen(function(body)
-			if body.protocolVersion ~= Config.protocolVersion then
-				local message = (
-					"Found a Rojo dev server, but it's using a different protocol version, and is incompatible." ..
-					"\nMake sure you have matching versions of both the Rojo plugin and server!" ..
-					"\n\nYour client is version %s, with protocol version %s. It expects server version %s." ..
-					"\nYour server is version %s, with protocol version %s." ..
-					"\n\nGo to https://github.com/rojo-rbx/rojo for more details."
-				):format(
-					Version.display(Config.version), Config.protocolVersion,
-					Config.expectedServerVersionString,
-					body.serverVersion, body.protocolVersion
-				)
-
-				return Promise.reject(message)
-			end
-
 			assert(validateApiInfo(body))
 
-			if body.expectedPlaceIds ~= nil then
+			return body
-				local foundId = false
+		end)
+		:andThen(rejectWrongPlaceId)
+		:andThen(function(body)
+			self.__serverId = body.serverId
 
-				for _, id in ipairs(body.expectedPlaceIds) do
+			return body
-					if id == game.PlaceId then
-						foundId = true
-						break
-					end
-				end
-
-				if not foundId then
-					local idList = {}
-					for _, id in ipairs(body.expectedPlaceIds) do
-						table.insert(idList, "- " .. tostring(id))
-					end
-
-					local message = (
-						"Found a Rojo server, but its project is set to only be used with a specific list of places." ..
-						"\nYour place ID is %s, but needs to be one of these:" ..
-						"\n%s" ..
-						"\n\nTo change this list, edit 'servePlaceIds' in roblox-project.json"
-					):format(
-						tostring(game.PlaceId),
-						table.concat(idList, "\n")
-					)
-
-					return Promise.reject(message)
-				end
-			end
-
-			self.serverId = body.serverId
-			self.partitionRoutes = body.partitions
-			self.rootInstanceId = body.rootInstanceId
 		end)
 end
 
 function ApiContext:read(ids)
-	local url = ("%s/api/read/%s"):format(self.baseUrl, table.concat(ids, ","))
+	local url = ("%s/api/read/%s"):format(self.__baseUrl, table.concat(ids, ","))
 
 	return Http.get(url)
 		:andThen(rejectFailedRequests)
-		:andThen(function(response)
+		:andThen(Http.Response.json)
-			local body = response:json()
+		:andThen(function(body)
-
+			if body.serverId ~= self.__serverId then
-			if body.serverId ~= self.serverId then
 				return Promise.reject("Server changed ID")
 			end
 
 			assert(validateApiRead(body))
 
-			self.messageCursor = body.messageCursor
+			self.__messageCursor = body.messageCursor
 
 			return body
 		end)
 end
 
 function ApiContext:retrieveMessages()
-	local url = ("%s/api/subscribe/%s"):format(self.baseUrl, self.messageCursor)
+	local url = ("%s/api/subscribe/%s"):format(self.__baseUrl, self.__messageCursor)
 
 	local function sendRequest()
 		return Http.get(url)
@@ -161,16 +142,15 @@ function ApiContext:retrieveMessages()
 
 	return sendRequest()
 		:andThen(rejectFailedRequests)
-		:andThen(function(response)
+		:andThen(Http.Response.json)
-			local body = response:json()
+		:andThen(function(body)
-
+			if body.serverId ~= self.__serverId then
-			if body.serverId ~= self.serverId then
 				return Promise.reject("Server changed ID")
 			end
 
 			assert(validateApiSubscribe(body))
 
-			self.messageCursor = body.messageCursor
+			self.__messageCursor = body.messageCursor
 
 			return body.messages
 		end)

plugin/src/Components/App.lua

@@ -4,15 +4,20 @@ local Plugin = Rojo.Plugin
 local Roact = require(Rojo.Roact)
 local Log = require(Rojo.Log)
 
+local ApiContext = require(Plugin.ApiContext)
 local Assets = require(Plugin.Assets)
 local Config = require(Plugin.Config)
 local DevSettings = require(Plugin.DevSettings)
-local Session = require(Plugin.Session)
+local Reconciler = require(Plugin.Reconciler)
+local ServeSession = require(Plugin.ServeSession)
 local Version = require(Plugin.Version)
 local preloadAssets = require(Plugin.preloadAssets)
+local strict = require(Plugin.strict)
 
 local ConnectPanel = require(Plugin.Components.ConnectPanel)
+local ConnectingPanel = require(Plugin.Components.ConnectingPanel)
 local ConnectionActivePanel = require(Plugin.Components.ConnectionActivePanel)
+local ErrorPanel = require(Plugin.Components.ErrorPanel)
 
 local e = Roact.createElement
 
@@ -52,26 +57,23 @@ local function checkUpgrade(plugin)
 	plugin:SetSetting("LastRojoVersion", Config.version)
 end
 
-local SessionStatus = {
+local AppStatus = strict("AppStatus", {
-	Disconnected = "Disconnected",
+	NotStarted = "NotStarted",
+	Connecting = "Connecting",
 	Connected = "Connected",
-}
+	Error = "Error",
-
-setmetatable(SessionStatus, {
-	__index = function(_, key)
-		error(("%q is not a valid member of SessionStatus"):format(tostring(key)), 2)
-	end,
 })
 
 local App = Roact.Component:extend("App")
 
 function App:init()
 	self:setState({
-		sessionStatus = SessionStatus.Disconnected,
+		appStatus = AppStatus.NotStarted,
+		errorMessage = nil,
 	})
 
 	self.signals = {}
-	self.currentSession = nil
+	self.serveSession = nil
 
 	self.displayedVersion = DevSettings:isEnabled()
 		and Config.codename
@@ -96,7 +98,7 @@ function App:init()
 		360, 190 -- Minimum size
 	)
 
-	self.dockWidget = self.props.plugin:CreateDockWidgetPluginGui("Rojo-0.5.x", widgetInfo)
+	self.dockWidget = self.props.plugin:CreateDockWidgetPluginGui("Rojo-" .. self.displayedVersion, widgetInfo)
 	self.dockWidget.Name = "Rojo " .. self.displayedVersion
 	self.dockWidget.Title = "Rojo " .. self.displayedVersion
 	self.dockWidget.AutoLocalize = false
@@ -107,56 +109,92 @@ function App:init()
 	end)
 end
 
+function App:startSession(address, port)
+	Log.trace("Starting new session")
+
+	local baseUrl = ("http://%s:%s"):format(address, port)
+	self.serveSession = ServeSession.new({
+		apiContext = ApiContext.new(baseUrl),
+		reconciler = Reconciler.new(),
+	})
+
+	self.serveSession:onStatusChanged(function(status, details)
+		if status == ServeSession.Status.Connecting then
+			self:setState({
+				appStatus = AppStatus.Connecting,
+			})
+		elseif status == ServeSession.Status.Connected then
+			self:setState({
+				appStatus = AppStatus.Connected,
+			})
+		elseif status == ServeSession.Status.Disconnected then
+			self.serveSession = nil
+
+			-- Details being present indicates that this
+			-- disconnection was from an error.
+			if details ~= nil then
+				Log.warn(tostring(details))
+
+				self:setState({
+					appStatus = AppStatus.Error,
+					errorMessage = tostring(details),
+				})
+			else
+				self:setState({
+					appStatus = AppStatus.NotStarted,
+				})
+			end
+		end
+	end)
+
+	self.serveSession:start()
+end
+
 function App:render()
 	local children
 
-	if self.state.sessionStatus == SessionStatus.Connected then
+	if self.state.appStatus == AppStatus.NotStarted then
+		children = {
+			ConnectPanel = e(ConnectPanel, {
+				startSession = function(address, port)
+					self:startSession(address, port)
+				end,
+				cancel = function()
+					Log.trace("Canceling session configuration")
+
+					self:setState({
+						appStatus = AppStatus.NotStarted,
+					})
+				end,
+			}),
+		}
+	elseif self.state.appStatus == AppStatus.Connecting then
+		children = {
+			ConnectingPanel = Roact.createElement(ConnectingPanel),
+		}
+	elseif self.state.appStatus == AppStatus.Connected then
 		children = {
 			ConnectionActivePanel = e(ConnectionActivePanel, {
 				stopSession = function()
 					Log.trace("Disconnecting session")
 
-					self.currentSession:disconnect()
+					self.serveSession:stop()
-					self.currentSession = nil
+					self.serveSession = nil
 					self:setState({
-						sessionStatus = SessionStatus.Disconnected,
+						appStatus = AppStatus.NotStarted,
 					})
 
 					Log.trace("Session terminated by user")
 				end,
 			}),
 		}
-	elseif self.state.sessionStatus == SessionStatus.Disconnected then
+	elseif self.state.appStatus == AppStatus.Error then
 		children = {
-			ConnectPanel = e(ConnectPanel, {
+			ErrorPanel = Roact.createElement(ErrorPanel, {
-				startSession = function(address, port)
+				errorMessage = self.state.errorMessage,
-					Log.trace("Starting new session")
+				onDismiss = function()
-
-					local success, session = Session.new({
-						address = address,
-						port = port,
-						onError = function(message)
-							Log.warn("Rojo session terminated because of an error:\n%s", tostring(message))
-							self.currentSession = nil
-
-							self:setState({
-								sessionStatus = SessionStatus.Disconnected,
-							})
-						end
-					})
-
-					if success then
-						self.currentSession = session
-						self:setState({
-							sessionStatus = SessionStatus.Connected,
-						})
-					end
-				end,
-				cancel = function()
-					Log.trace("Canceling session configuration")
-
 					self:setState({
-						sessionStatus = SessionStatus.Disconnected,
+						appStatus = AppStatus.NotStarted,
 					})
 				end,
 			}),
@@ -176,9 +214,9 @@ function App:didMount()
 end
 
 function App:willUnmount()
-	if self.currentSession ~= nil then
+	if self.serveSession ~= nil then
-		self.currentSession:disconnect()
+		self.serveSession:stop()
-		self.currentSession = nil
+		self.serveSession = nil
 	end
 
 	for _, signal in pairs(self.signals) do

plugin/src/Components/ConnectingPanel.lua

@@ -0,0 +1,34 @@
+local Roact = require(script:FindFirstAncestor("Rojo").Roact)
+
+local Plugin = script:FindFirstAncestor("Plugin")
+
+local Theme = require(Plugin.Theme)
+
+local Panel = require(Plugin.Components.Panel)
+local FitText = require(Plugin.Components.FitText)
+
+local e = Roact.createElement
+
+local ConnectingPanel = Roact.Component:extend("ConnectingPanel")
+
+function ConnectingPanel:render()
+	return e(Panel, nil, {
+		Layout = Roact.createElement("UIListLayout", {
+			HorizontalAlignment = Enum.HorizontalAlignment.Center,
+			VerticalAlignment = Enum.VerticalAlignment.Center,
+			SortOrder = Enum.SortOrder.LayoutOrder,
+			Padding = UDim.new(0, 8),
+		}),
+
+		Text = e(FitText, {
+			Padding = Vector2.new(12, 6),
+			Font = Theme.ButtonFont,
+			TextSize = 18,
+			Text = "Connecting...",
+			TextColor3 = Theme.PrimaryColor,
+			BackgroundTransparency = 1,
+		}),
+	})
+end
+
+return ConnectingPanel

plugin/src/Components/ErrorPanel.lua

@@ -0,0 +1,51 @@
+local Roact = require(script:FindFirstAncestor("Rojo").Roact)
+
+local Plugin = script:FindFirstAncestor("Plugin")
+
+local Theme = require(Plugin.Theme)
+
+local Panel = require(Plugin.Components.Panel)
+local FitText = require(Plugin.Components.FitText)
+local FormButton = require(Plugin.Components.FormButton)
+
+local e = Roact.createElement
+
+local ErrorPanel = Roact.Component:extend("ErrorPanel")
+
+function ErrorPanel:render()
+	local errorMessage = self.props.errorMessage
+	local onDismiss = self.props.onDismiss
+
+	return e(Panel, nil, {
+		Layout = Roact.createElement("UIListLayout", {
+			HorizontalAlignment = Enum.HorizontalAlignment.Center,
+			VerticalAlignment = Enum.VerticalAlignment.Center,
+			SortOrder = Enum.SortOrder.LayoutOrder,
+			Padding = UDim.new(0, 8),
+		}),
+
+		Text = e(FitText, {
+			LayoutOrder = 1,
+			FitAxis = "Y",
+			Size = UDim2.new(1, 0, 0, 0),
+			Padding = Vector2.new(12, 6),
+			Font = Theme.ButtonFont,
+			TextSize = 18,
+			Text = errorMessage,
+			TextWrap = true,
+			TextColor3 = Theme.PrimaryColor,
+			BackgroundTransparency = 1,
+		}),
+
+		DismissButton = e(FormButton, {
+			layoutOrder = 2,
+			text = "Dismiss",
+			secondary = true,
+			onClick = function()
+				onDismiss()
+			end,
+		}),
+	})
+end
+
+return ErrorPanel

plugin/src/Components/FitText.lua

@@ -9,6 +9,7 @@ local e = Roact.createElement
 local FitText = Roact.Component:extend("FitText")
 
 function FitText:init()
+	self.ref = Roact.createRef()
 	self.sizeBinding, self.setSize = Roact.createBinding(UDim2.new())
 end
 
@@ -16,10 +17,15 @@ function FitText:render()
 	local kind = self.props.Kind or "TextLabel"
 
 	local containerProps = Dictionary.merge(self.props, {
+		FitAxis = Dictionary.None,
 		Kind = Dictionary.None,
 		Padding = Dictionary.None,
 		MinSize = Dictionary.None,
-		Size = self.sizeBinding
+		Size = self.sizeBinding,
+		[Roact.Ref] = self.ref,
+		[Roact.Change.AbsoluteSize] = function()
+			self:updateTextMeasurements()
+		end
 	})
 
 	return e(kind, containerProps)
@@ -36,15 +42,45 @@ end
 function FitText:updateTextMeasurements()
 	local minSize = self.props.MinSize or Vector2.new(0, 0)
 	local padding = self.props.Padding or Vector2.new(0, 0)
+	local fitAxis = self.props.FitAxis or "XY"
+	local baseSize = self.props.Size
 
 	local text = self.props.Text or ""
 	local font = self.props.Font or Enum.Font.Legacy
 	local textSize = self.props.TextSize or 12
 
-	local measuredText = TextService:GetTextSize(text, textSize, font, Vector2.new(9e6, 9e6))
+	local containerSize = self.ref.current.AbsoluteSize
-	local totalSize = UDim2.new(
+
-		0, math.max(minSize.X, padding.X * 2 + measuredText.X),
+	local textBounds
-		0, math.max(minSize.Y, padding.Y * 2 + measuredText.Y))
+
+	if fitAxis == "XY" then
+		textBounds = Vector2.new(9e6, 9e6)
+	elseif fitAxis == "X" then
+		textBounds = Vector2.new(9e6, containerSize.Y - padding.Y * 2)
+	elseif fitAxis == "Y" then
+		textBounds = Vector2.new(containerSize.X - padding.X * 2, 9e6)
+	end
+
+	local measuredText = TextService:GetTextSize(text, textSize, font, textBounds)
+
+	local computedX = math.max(minSize.X, padding.X * 2 + measuredText.X)
+	local computedY = math.max(minSize.Y, padding.Y * 2 + measuredText.Y)
+
+	local totalSize
+
+	if fitAxis == "XY" then
+		totalSize = UDim2.new(
+			0, computedX,
+			0, computedY)
+	elseif fitAxis == "X" then
+		totalSize = UDim2.new(
+			0, computedX,
+			baseSize.Y.Scale, baseSize.Y.Offset)
+	elseif fitAxis == "Y" then
+		totalSize = UDim2.new(
+			baseSize.X.Scale, baseSize.X.Offset,
+			0, computedY)
+	end
 
 	self.setSize(totalSize)
 end

plugin/src/Reconciler.lua

@@ -1,239 +1,253 @@
+--[[
+	This module defines the meat of the Rojo plugin and how it manages tracking
+	and mutating the Roblox DOM.
+]]
+
+local RbxDom = require(script.Parent.Parent.RbxDom)
 local t = require(script.Parent.Parent.t)
-local Log = require(script.Parent.Parent.Log)
 
 local InstanceMap = require(script.Parent.InstanceMap)
-local setCanonicalProperty = require(script.Parent.setCanonicalProperty)
-local rojoValueToRobloxValue = require(script.Parent.rojoValueToRobloxValue)
 local Types = require(script.Parent.Types)
+local setCanonicalProperty = require(script.Parent.setCanonicalProperty)
 
-local function setParent(instance, newParent)
+--[[
+	This interface represents either a patch created by the hydrate method, or a
+	patch returned from the API.
+
+	This type should be a subset of Types.ApiInstanceUpdate.
+]]
+local IPatch = t.interface({
+	removed = t.array(t.union(Types.RbxId, t.Instance)),
+	added = t.map(Types.RbxId, Types.ApiInstance),
+	updated = t.array(Types.ApiInstanceUpdate),
+})
+
+--[[
+	Attempt to safely set the parent of an instance.
+
+	This function will always succeed, even if the actual set failed. This is
+	important for some types like services that will throw even if their current
+	parent is already set to the requested parent.
+
+	TODO: See if we can eliminate this by being more nuanced with property
+	assignment?
+]]
+local function safeSetParent(instance, newParent)
 	pcall(function()
 		instance.Parent = newParent
 	end)
 end
 
+--[[
+	Similar to setting Parent, some instances really don't like being renamed.
+
+	TODO: Should we be throwing away these results or can we be more careful?
+]]
+local function safeSetName(instance, name)
+	pcall(function()
+		instance.Name = name
+	end)
+end
+
 local Reconciler = {}
 Reconciler.__index = Reconciler
 
 function Reconciler.new()
 	local self = {
-		instanceMap = InstanceMap.new(),
+		-- Tracks all of the instances known by the reconciler by ID.
+		__instanceMap = InstanceMap.new(),
 	}
 
 	return setmetatable(self, Reconciler)
 end
 
-function Reconciler:applyUpdate(requestedIds, virtualInstancesById)
+--[[
-	-- This function may eventually be asynchronous; it will require calls to
+	See Reconciler:__hydrateInternal().
-	-- the server to resolve instances that don't exist yet.
+]]
-	local visitedIds = {}
+function Reconciler:hydrate(apiInstances, id, instance)
+	local hydratePatch = {
+		removed = {},
+		added = {},
+		updated = {},
+	}
 
-	for _, id in ipairs(requestedIds) do
+	self:__hydrateInternal(apiInstances, id, instance, hydratePatch)
-		self:__applyUpdatePiece(id, visitedIds, virtualInstancesById)
+
-	end
+	return hydratePatch
 end
 
-local reconcileSchema = Types.ifEnabled(t.tuple(
+--[[
-	t.map(t.string, Types.VirtualInstance),
+	Transforms a value encoded by rbx_dom_weak on the server side into a value
-	t.string,
+	usable by Rojo's reconciler, potentially using RbxDom.
+]]
+function Reconciler:__decodeApiValue(apiValue)
+	assert(Types.ApiValue(apiValue))
+
+	-- Refs are represented as IDs in the same space that Rojo's protocol uses.
+	if apiValue.Type == "Ref" then
+		-- TODO: This ref could be pointing at an instance we haven't created
+		-- yet!
+
+		return self.__instanceMap.fromIds[apiValue.Value]
+	end
+
+	local success, decodedValue = RbxDom.EncodedValue.decode(apiValue)
+
+	if not success then
+		error(decodedValue, 2)
+	end
+
+	return decodedValue
+end
+
+--[[
+	Constructs an instance from an ApiInstance without any of its children.
+]]
+local reifySingleInstanceSchema = Types.ifEnabled(t.tuple(
+	Types.ApiInstance
+))
+function Reconciler:__reifySingleInstance(apiInstance)
+	assert(reifySingleInstanceSchema(apiInstance))
+
+	-- Instance.new can fail if we're passing in something that can't be
+	-- created, like a service, something enabled with a feature flag, or
+	-- something that requires higher security than we have.
+	local ok, instance = pcall(Instance.new, apiInstance.ClassName)
+	if not ok then
+		return false, instance
+	end
+
+	-- TODO: When can setting Name fail here?
+	safeSetName(instance, apiInstance.Name)
+
+	for key, value in pairs(apiInstance.Properties) do
+		setCanonicalProperty(instance, key, self:__decodeApiValue(value))
+	end
+
+	return instance
+end
+
+--[[
+	Construct an instance and all of its descendants, parent it to the given
+	instance, and insert it into the reconciler's internal state.
+]]
+local reifyInstanceSchema = Types.ifEnabled(t.tuple(
+	t.map(Types.RbxId, Types.VirtualInstance),
+	Types.RbxId,
 	t.Instance
 ))
+function Reconciler:__reifyInstance(apiInstances, id, parentInstance)
+	assert(reifyInstanceSchema(apiInstances, id, parentInstance))
+
+	local apiInstance = apiInstances[id]
+	local ok, instance = self:__reifySingleInstance(apiInstance)
+
+	-- TODO: Propagate this error upward to handle it elsewhere?
+	if not ok then
+		error(("Couldn't create an instance of type %q, a child of %s"):format(
+			apiInstance.ClassName,
+			parentInstance:GetFullName()
+		))
+	end
+
+	for _, childId in ipairs(apiInstance.Children) do
+		self:__reify(apiInstances, childId, instance)
+	end
+
+	safeSetParent(instance, parentInstance)
+	self.__instanceMap:insert(id, instance)
+
+	return instance
+end
+
 --[[
-	Update an existing instance, including its properties and children, to match
+	Populates the reconciler's internal state, maps IDs to instances that the
-	the given information.
+	Rojo plugin knows about, and generates a patch that would update the Roblox
+	tree to match Rojo's view of the tree.
 ]]
-function Reconciler:reconcile(virtualInstancesById, id, instance)
+local hydrateSchema = Types.ifEnabled(t.tuple(
-	assert(reconcileSchema(virtualInstancesById, id, instance))
+	t.map(Types.RbxId, Types.VirtualInstance),
+	Types.RbxId,
+	t.Instance,
+	IPatch
+))
+function Reconciler:__hydrateInternal(apiInstances, id, instance, hydratePatch)
+	assert(hydrateSchema(apiInstances, id, instance, hydratePatch))
 
-	local virtualInstance = virtualInstancesById[id]
+	local apiInstance = apiInstances[id]
 
-	-- If an instance changes ClassName, we assume it's very different. That's
+	local function markIdAdded(id)
-	-- not always the case!
+		local apiInstance = apiInstances[id]
-	if virtualInstance.ClassName ~= instance.ClassName then
+		hydratePatch.added[id] = apiInstance
-		Log.trace("Switching to reify for %s because ClassName is different", instance:GetFullName())
 
-		-- TODO: Preserve existing children instead?
+		for _, childId in ipairs(apiInstance.Children) do
-		local parent = instance.Parent
+			markIdAdded(childId)
-		self.instanceMap:destroyId(id)
+		end
-		return self:__reify(virtualInstancesById, id, parent)
 	end
 
-	self.instanceMap:insert(id, instance)
+	-- TODO: Measure differences in properties and add them to
-
+	-- hydratePatch.updates
-	-- Some instances don't like being named, even if their name already matches
-	setCanonicalProperty(instance, "Name", virtualInstance.Name)
-
-	for key, value in pairs(virtualInstance.Properties) do
-		setCanonicalProperty(instance, key, rojoValueToRobloxValue(value))
-	end
 
 	local existingChildren = instance:GetChildren()
 
-	local unvisitedExistingChildren = {}
+	-- For each existing child, we'll track whether it's been paired with an
-	for _, child in ipairs(existingChildren) do
+	-- instance that the Rojo server knows about.
-		unvisitedExistingChildren[child] = true
+	local isExistingChildVisited = {}
+	for i = 1, #existingChildren do
+		isExistingChildVisited[i] = false
 	end
 
-	for _, childId in ipairs(virtualInstance.Children) do
+	for _, childId in ipairs(apiInstance.Children) do
-		local childData = virtualInstancesById[childId]
+		local apiChild = apiInstances[childId]
 
-		local existingChildInstance
+		local childInstance
-		for instance in pairs(unvisitedExistingChildren) do
-			local ok, name, className = pcall(function()
-				return instance.Name, instance.ClassName
-			end)
 
-			if ok then
+		for childIndex, instance in ipairs(existingChildren) do
-				if name == childData.Name and className == childData.ClassName then
+			if not isExistingChildVisited[childIndex] then
-					existingChildInstance = instance
+				-- We guard accessing Name and ClassName in order to avoid
+				-- tripping over children of DataModel that Rojo won't have
+				-- permissions to access at all.
+				local ok, name, className = pcall(function()
+					return instance.Name, instance.ClassName
+				end)
+
+				-- This rule is very conservative and could be loosened in the
+				-- future, or more heuristics could be introduced.
+				if ok and name == apiChild.Name and className == apiChild.ClassName then
+					childInstance = instance
+					isExistingChildVisited[childIndex] = true
 					break
 				end
 			end
 		end
 
-		if existingChildInstance ~= nil then
+		if childInstance ~= nil then
-			unvisitedExistingChildren[existingChildInstance] = nil
+			-- We found an instance that matches the instance from the API, yay!
-			self:reconcile(virtualInstancesById, childId, existingChildInstance)
+			self:__hydrateInternal(apiInstances, childId, childInstance, hydratePatch)
 		else
-			Log.trace(
+			markIdAdded(childId)
-				"Switching to reify for %s.%s because it does not exist",
-				instance:GetFullName(),
-				virtualInstancesById[childId].Name
-			)
-
-			self:__reify(virtualInstancesById, childId, instance)
 		end
 	end
 
-	local shouldClearUnknown = self:__shouldClearUnknownChildren(virtualInstance)
+	-- Any unvisited children at this point aren't known by Rojo and we can
-
+	-- destroy them unless the user has explicitly asked us to preserve children
-	for existingChildInstance in pairs(unvisitedExistingChildren) do
+	-- of this instance.
-		local childId = self.instanceMap.fromInstances[existingChildInstance]
+	local shouldClearUnknown = self:__shouldClearUnknownChildren(apiInstance)
-
+	if shouldClearUnknown then
-		if childId == nil then
+		for childIndex, visited in ipairs(isExistingChildVisited) do
-			if shouldClearUnknown then
+			if not visited then
-				existingChildInstance:Destroy()
+				table.insert(hydratePatch.removedInstances, existingChildren[childIndex])
 			end
-		else
-			self.instanceMap:destroyInstance(existingChildInstance)
 		end
 	end
-
-	-- The root instance of a project won't have a parent, like the DataModel,
-	-- so we need to be careful here.
-	if virtualInstance.Parent ~= nil then
-		local parent = self.instanceMap.fromIds[virtualInstance.Parent]
-
-		if parent == nil then
-			Log.info("Instance %s wanted parent of %s", tostring(id), tostring(virtualInstance.Parent))
-			error("Rojo bug: During reconciliation, an instance referred to an instance ID as parent that does not exist.")
-		end
-
-		-- Some instances, like services, don't like having their Parent
-		-- property poked, even if we're setting it to the same value.
-		setParent(instance, parent)
-	end
-
-	return instance
 end
 
-function Reconciler:__shouldClearUnknownChildren(virtualInstance)
+function Reconciler:__shouldClearUnknownChildren(apiInstance)
-	if virtualInstance.Metadata ~= nil then
+	if apiInstance.Metadata ~= nil then
-		return not virtualInstance.Metadata.ignoreUnknownInstances
+		return not apiInstance.Metadata.ignoreUnknownInstances
 	else
 		return true
 	end
 end
 
-local reifySchema = Types.ifEnabled(t.tuple(
-	t.map(t.string, Types.VirtualInstance),
-	t.string,
-	t.Instance
-))
-
-function Reconciler:__reify(virtualInstancesById, id, parent)
-	assert(reifySchema(virtualInstancesById, id, parent))
-
-	local virtualInstance = virtualInstancesById[id]
-
-	local ok, instance = pcall(function()
-		return Instance.new(virtualInstance.ClassName)
-	end)
-
-	if not ok then
-		error(("Couldn't create an Instance of type %q, a child of %s"):format(
-			virtualInstance.ClassName,
-			parent:GetFullName()
-		))
-	end
-
-	for key, value in pairs(virtualInstance.Properties) do
-		setCanonicalProperty(instance, key, rojoValueToRobloxValue(value))
-	end
-
-	setCanonicalProperty(instance, "Name", virtualInstance.Name)
-
-	for _, childId in ipairs(virtualInstance.Children) do
-		self:__reify(virtualInstancesById, childId, instance)
-	end
-
-	setParent(instance, parent)
-	self.instanceMap:insert(id, instance)
-
-	return instance
-end
-
-local applyUpdatePieceSchema = Types.ifEnabled(t.tuple(
-	t.string,
-	t.map(t.string, t.boolean),
-	t.map(t.string, Types.VirtualInstance)
-))
-
-function Reconciler:__applyUpdatePiece(id, visitedIds, virtualInstancesById)
-	assert(applyUpdatePieceSchema(id, visitedIds, virtualInstancesById))
-
-	if visitedIds[id] then
-		return
-	end
-
-	visitedIds[id] = true
-
-	local virtualInstance = virtualInstancesById[id]
-	local instance = self.instanceMap.fromIds[id]
-
-	-- The instance was deleted in this update
-	if virtualInstance == nil then
-		self.instanceMap:destroyId(id)
-		return
-	end
-
-	-- An instance we know about was updated
-	if instance ~= nil then
-		self:reconcile(virtualInstancesById, id, instance)
-		return instance
-	end
-
-	-- If the instance's parent already exists, we can stick it there
-	local parentInstance = self.instanceMap.fromIds[virtualInstance.Parent]
-	if parentInstance ~= nil then
-		self:__reify(virtualInstancesById, id, parentInstance)
-		return
-	end
-
-	-- Otherwise, we can check if this response payload contained the parent and
-	-- work from there instead.
-	local parentData = virtualInstancesById[virtualInstance.Parent]
-	if parentData ~= nil then
-		if visitedIds[virtualInstance.Parent] then
-			error("Rojo bug: An instance was present and marked as visited but its instance was missing")
-		end
-
-		self:__applyUpdatePiece(virtualInstance.Parent, visitedIds, virtualInstancesById)
-		return
-	end
-
-	Log.trace("Instance ID %s, parent ID %s", tostring(id), tostring(virtualInstance.Parent))
-	error("Rojo NYI: Instances with parents that weren't mentioned in an update payload")
-end
-
 return Reconciler

plugin/src/Reconciler.spec.lua

@@ -1,218 +0,0 @@
-local Reconciler = require(script.Parent.Reconciler)
-
-return function()
-	it("should leave instances alone if there's nothing specified", function()
-		local instance = Instance.new("Folder")
-		instance.Name = "TestFolder"
-
-		local instanceId = "test-id"
-		local virtualInstancesById = {
-			[instanceId] = {
-				Name = "TestFolder",
-				ClassName = "Folder",
-				Children = {},
-				Properties = {},
-			},
-		}
-
-		local reconciler = Reconciler.new()
-		reconciler:reconcile(virtualInstancesById, instanceId, instance)
-	end)
-
-	it("should assign names from virtual instances", function()
-		local instance = Instance.new("Folder")
-		instance.Name = "InitialName"
-
-		local instanceId = "test-id"
-		local virtualInstancesById = {
-			[instanceId] = {
-				Name = "NewName",
-				ClassName = "Folder",
-				Children = {},
-				Properties = {},
-			},
-		}
-
-		local reconciler = Reconciler.new()
-		reconciler:reconcile(virtualInstancesById, instanceId, instance)
-
-		expect(instance.Name).to.equal("NewName")
-	end)
-
-	it("should assign properties from virtual instances", function()
-		local instance = Instance.new("IntValue")
-		instance.Name = "TestValue"
-		instance.Value = 5
-
-		local instanceId = "test-id"
-		local virtualInstancesById = {
-			[instanceId] = {
-				Name = "TestValue",
-				ClassName = "IntValue",
-				Children = {},
-				Properties = {
-					Value = {
-						Type = "Int32",
-						Value = 9
-					}
-				},
-			},
-		}
-
-		local reconciler = Reconciler.new()
-		reconciler:reconcile(virtualInstancesById, instanceId, instance)
-
-		expect(instance.Value).to.equal(9)
-	end)
-
-	it("should wipe unknown children by default", function()
-		local parent = Instance.new("Folder")
-		parent.Name = "Parent"
-
-		local child = Instance.new("Folder")
-		child.Name = "Child"
-
-		local parentId = "test-id"
-		local virtualInstancesById = {
-			[parentId] = {
-				Name = "Parent",
-				ClassName = "Folder",
-				Children = {},
-				Properties = {},
-			},
-		}
-
-		local reconciler = Reconciler.new()
-		reconciler:reconcile(virtualInstancesById, parentId, parent)
-
-		expect(#parent:GetChildren()).to.equal(0)
-	end)
-
-	it("should preserve unknown children if ignoreUnknownInstances is set", function()
-		local parent = Instance.new("Folder")
-		parent.Name = "Parent"
-
-		local child = Instance.new("Folder")
-		child.Parent = parent
-		child.Name = "Child"
-
-		local parentId = "test-id"
-		local virtualInstancesById = {
-			[parentId] = {
-				Name = "Parent",
-				ClassName = "Folder",
-				Children = {},
-				Properties = {},
-				Metadata = {
-					ignoreUnknownInstances = true,
-				},
-			},
-		}
-
-		local reconciler = Reconciler.new()
-		reconciler:reconcile(virtualInstancesById, parentId, parent)
-
-		expect(child.Parent).to.equal(parent)
-		expect(#parent:GetChildren()).to.equal(1)
-	end)
-
-	it("should remove known removed children", function()
-		local parent = Instance.new("Folder")
-		parent.Name = "Parent"
-
-		local child = Instance.new("Folder")
-		child.Parent = parent
-		child.Name = "Child"
-
-		local parentId = "parent-id"
-		local childId = "child-id"
-
-		local reconciler = Reconciler.new()
-
-		local virtualInstancesById = {
-			[parentId] = {
-				Name = "Parent",
-				ClassName = "Folder",
-				Children = {childId},
-				Properties = {},
-			},
-			[childId] = {
-				Name = "Child",
-				ClassName = "Folder",
-				Children = {},
-				Properties = {},
-			},
-		}
-		reconciler:reconcile(virtualInstancesById, parentId, parent)
-
-		expect(child.Parent).to.equal(parent)
-		expect(#parent:GetChildren()).to.equal(1)
-
-		local newVirtualInstances = {
-			[parentId] = {
-				Name = "Parent",
-				ClassName = "Folder",
-				Children = {},
-				Properties = {},
-			},
-			[childId] = nil,
-		}
-		reconciler:reconcile(newVirtualInstances, parentId, parent)
-
-		expect(child.Parent).to.equal(nil)
-		expect(#parent:GetChildren()).to.equal(0)
-	end)
-
-	it("should remove known removed children if ignoreUnknownInstances is set", function()
-		local parent = Instance.new("Folder")
-		parent.Name = "Parent"
-
-		local child = Instance.new("Folder")
-		child.Parent = parent
-		child.Name = "Child"
-
-		local parentId = "parent-id"
-		local childId = "child-id"
-
-		local reconciler = Reconciler.new()
-
-		local virtualInstancesById = {
-			[parentId] = {
-				Name = "Parent",
-				ClassName = "Folder",
-				Children = {childId},
-				Properties = {},
-				Metadata = {
-					ignoreUnknownInstances = true,
-				},
-			},
-			[childId] = {
-				Name = "Child",
-				ClassName = "Folder",
-				Children = {},
-				Properties = {},
-			},
-		}
-		reconciler:reconcile(virtualInstancesById, parentId, parent)
-
-		expect(child.Parent).to.equal(parent)
-		expect(#parent:GetChildren()).to.equal(1)
-
-		local newVirtualInstances = {
-			[parentId] = {
-				Name = "Parent",
-				ClassName = "Folder",
-				Children = {},
-				Properties = {},
-				Metadata = {
-					ignoreUnknownInstances = true,
-				},
-			},
-			[childId] = nil,
-		}
-		reconciler:reconcile(newVirtualInstances, parentId, parent)
-
-		expect(child.Parent).to.equal(nil)
-		expect(#parent:GetChildren()).to.equal(0)
-	end)
-end

plugin/src/ServeSession.lua

@@ -0,0 +1,99 @@
+local t = require(script.Parent.Parent.t)
+
+local strict = require(script.Parent.strict)
+
+local Status = strict("Session.Status", {
+	NotStarted = "NotStarted",
+	Connecting = "Connecting",
+	Connected = "Connected",
+	Disconnected = "Disconnected",
+})
+
+local function DEBUG_printPatch(patch)
+	local HttpService = game:GetService("HttpService")
+
+
+	for removed in ipairs(patch.removed) do
+		print("Remove:", removed)
+	end
+
+	for id, added in pairs(patch.added) do
+		print("Add:", id, HttpService:JSONEncode(added))
+	end
+
+	for updated in ipairs(patch.updated) do
+		print("Update:", HttpService:JSONEncode(updated))
+	end
+end
+
+local ServeSession = {}
+ServeSession.__index = ServeSession
+
+ServeSession.Status = Status
+
+local validateServeOptions = t.strictInterface({
+	apiContext = t.table,
+	reconciler = t.table,
+})
+
+function ServeSession.new(options)
+	assert(validateServeOptions(options))
+
+	local self = {
+		__status = Status.NotStarted,
+		__apiContext = options.apiContext,
+		__reconciler = options.reconciler,
+		__statusChangedCallback = nil,
+	}
+
+	setmetatable(self, ServeSession)
+
+	return self
+end
+
+function ServeSession:onStatusChanged(callback)
+	self.__statusChangedCallback = callback
+end
+
+function ServeSession:start()
+	self:__setStatus(Status.Connecting)
+
+	self.__apiContext:connect()
+		:andThen(function(serverInfo)
+			self:__setStatus(Status.Connected)
+
+			local rootInstanceId = serverInfo.rootInstanceId
+
+			return self.__apiContext:read({ rootInstanceId })
+				:andThen(function(readResponseBody)
+					local hydratePatch = self.__reconciler:hydrate(
+						readResponseBody.instances,
+						rootInstanceId,
+						game
+					)
+
+					DEBUG_printPatch(hydratePatch)
+
+					-- TODO: Apply the patch generated by hydration. We should
+					-- eventually prompt the user about this since it's a
+					-- conflict between Rojo and their current place state.
+				end)
+		end)
+		:catch(function(err)
+			self:__setStatus(Status.Disconnected, err)
+		end)
+end
+
+function ServeSession:stop()
+	self:__setStatus(Status.Disconnected)
+end
+
+function ServeSession:__setStatus(status, detail)
+	self.__status = status
+
+	if self.__statusChangedCallback ~= nil then
+		self.__statusChangedCallback(status, detail)
+	end
+end
+
+return ServeSession

plugin/src/Session.lua

@@ -1,95 +0,0 @@
-local Promise = require(script.Parent.Parent.Promise)
-
-local ApiContext = require(script.Parent.ApiContext)
-local Reconciler = require(script.Parent.Reconciler)
-
-local Session = {}
-Session.__index = Session
-
-function Session.new(config)
-	local baseUrl = ("http://%s:%s"):format(config.address, config.port)
-	local apiContext = ApiContext.new(baseUrl)
-
-	local self = {
-		onError = config.onError,
-		disconnected = false,
-		reconciler = Reconciler.new(),
-		apiContext = apiContext,
-	}
-
-	apiContext:connect()
-		:andThen(function()
-			if self.disconnected then
-				return
-			end
-
-			return apiContext:read({apiContext.rootInstanceId})
-		end)
-		:andThen(function(response)
-			if self.disconnected then
-				return
-			end
-
-			self.reconciler:reconcile(response.instances, apiContext.rootInstanceId, game)
-			return self:__processMessages()
-		end)
-		:catch(function(message)
-			self.disconnected = true
-			self.onError(message)
-		end)
-
-	return not self.disconnected, setmetatable(self, Session)
-end
-
-function Session:__processMessages()
-	if self.disconnected then
-		return Promise.resolve()
-	end
-
-	return self.apiContext:retrieveMessages()
-		:andThen(function(messages)
-			local promise = Promise.resolve(nil)
-
-			for _, message in ipairs(messages) do
-				promise = promise:andThen(function()
-					return self:__onMessage(message)
-				end)
-			end
-
-			return promise
-		end)
-		:andThen(function()
-			return self:__processMessages()
-		end)
-end
-
-function Session:__onMessage(message)
-	if self.disconnected then
-		return Promise.resolve()
-	end
-
-	local requestedIds = {}
-
-	for _, id in ipairs(message.added) do
-		table.insert(requestedIds, id)
-	end
-
-	for _, id in ipairs(message.updated) do
-		table.insert(requestedIds, id)
-	end
-
-	for _, id in ipairs(message.removed) do
-		table.insert(requestedIds, id)
-	end
-
-	return self.apiContext:read(requestedIds)
-		:andThen(function(response)
-			return self.reconciler:applyUpdate(requestedIds, response.instances)
-		end)
-end
-
-function Session:disconnect()
-	self.disconnected = true
-end
-
-return Session

plugin/src/Types.lua

@@ -84,6 +84,7 @@ return strict("Types", {
 	ApiError = ApiError,
 
 	ApiInstance = ApiInstance,
+	ApiInstanceUpdate = ApiInstanceUpdate,
 	ApiInstanceMetadata = ApiInstanceMetadata,
 	ApiSubscribeMessage = ApiSubscribeMessage,
 	ApiValue = ApiValue,

plugin/src/rojoValueToRobloxValue.lua

@@ -1,19 +0,0 @@
-local RbxDom = require(script:FindFirstAncestor("Rojo").RbxDom)
-
-local function rojoValueToRobloxValue(value)
-	-- TODO: Manually decode this value by looking up its GUID The Rojo server
-	-- doesn't give us valid ref values yet, so this isn't important yet.
-	if value.Type == "Ref" then
-		return nil
-	end
-
-	local success, decodedValue = RbxDom.EncodedValue.decode(value)
-
-	if not success then
-		error(decodedValue, 2)
-	end
-
-	return decodedValue
-end
-
-return rojoValueToRobloxValue

plugin/src/rojoValueToRobloxValue.spec.lua

@@ -1,40 +0,0 @@
-local rojoValueToRobloxValue = require(script.Parent.rojoValueToRobloxValue)
-
-return function()
-	it("should convert primitives", function()
-		local inputString = {
-			Type = "String",
-			Value = "Hello, world!",
-		}
-
-		local inputFloat32 = {
-			Type = "Float32",
-			Value = 12341.512,
-		}
-
-		expect(rojoValueToRobloxValue(inputString)).to.equal(inputString.Value)
-		expect(rojoValueToRobloxValue(inputFloat32)).to.equal(inputFloat32.Value)
-	end)
-
-	it("should convert properties with direct constructors", function()
-		local inputColor3 = {
-			Type = "Color3",
-			Value = {0, 1, 0.5},
-		}
-		local outputColor3 = Color3.new(0, 1, 0.5)
-
-		local inputCFrame = {
-			Type = "CFrame",
-			Value = {
-				1, 2, 3,
-				4, 5, 6,
-				7, 8, 9,
-				10, 11, 12,
-			},
-		}
-		local outputCFrame = CFrame.new(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12)
-
-		expect(rojoValueToRobloxValue(inputColor3)).to.equal(outputColor3)
-		expect(rojoValueToRobloxValue(inputCFrame)).to.equal(outputCFrame)
-	end)
-end