Release 0.4.6

Closes #67.

CHANGES.md

@@ -1,7 +1,15 @@
 # Rojo Change Log
 
-## Current Master
-*No changes*
+## Current master
+* *No changes*
+
+## 0.4.6 (May 21, 2018)
+* Rojo handles being restarted by Roblox Studio more gracefully ([#67](https://github.com/LPGhatguy/rojo/issues/67))
+* Folders should no longer get collapsed when syncing occurs.
+* **Significant** robustness improvements with regards to caching.
+    * **This should catch all existing script duplication bugs.**
+    * If there are any bugs with script duplication or caching in the future, restarting the Rojo server process will fix them for that session.
+* Fixed message in plugin not being prefixed with `Rojo: `.
 
 ## 0.4.5 (May 1, 2018)
 * Rojo messages are now prefixed with `Rojo: ` to make them stand out in the output more.

README.md

@@ -8,7 +8,7 @@
     <a href="https://travis-ci.org/LPGhatguy/rojo">
         <img src="https://api.travis-ci.org/LPGhatguy/rojo.svg?branch=master" alt="Travis-CI Build Status" />
     </a>
-    <img src="https://img.shields.io/badge/latest_version-0.4.5-brightgreen.svg" alt="Current server version" />
+    <img src="https://img.shields.io/badge/latest_version-0.4.6-brightgreen.svg" alt="Current server version" />
     <a href="https://lpghatguy.github.io/rojo">
         <img src="https://img.shields.io/badge/documentation-website-brightgreen.svg" alt="Rojo Documentation" />
     </a>

plugin/src/Config.lua

@@ -1,6 +1,6 @@
 return {
 	pollingRate = 0.2,
-	version = {0, 4, 5},
+	version = {0, 4, 6},
 	expectedServerVersionString = "0.4.x",
 	protocolVersion = 1,
 	dev = false,

plugin/src/Plugin.lua

@@ -1,9 +1,12 @@
+local CoreGui = game:GetService("CoreGui")
+
 local Promise = require(script.Parent.Parent.modules.Promise)
 
 local Config = require(script.Parent.Config)
 local Http = require(script.Parent.Http)
 local Api = require(script.Parent.Api)
 local Reconciler = require(script.Parent.Reconciler)
+local Version = require(script.Parent.Version)
 
 local function collectMatch(source, pattern)
 	local result = {}
@@ -35,9 +38,23 @@ function Plugin.new()
 	setmetatable(self, Plugin)
 
 	do
+		local uiName = ("Rojo %s UI"):format(Version.display(Config.version))
+
+		if Config.dev then
+			uiName = "Rojo Dev UI"
+		end
+
+		-- If there's an existing Rojo UI, like from a Roblox plugin upgrade
+		-- that wasn't Rojo, make sure we clean it up.
+		local existingUi = CoreGui:FindFirstChild(uiName)
+
+		if existingUi ~= nil then
+			existingUi:Destroy()
+		end
+
 		local screenGui = Instance.new("ScreenGui")
-		screenGui.Name = "Rojo UI"
-		screenGui.Parent = game.CoreGui
+		screenGui.Name = uiName
+		screenGui.Parent = CoreGui
 		screenGui.DisplayOrder = -1
 		screenGui.Enabled = false
 
@@ -54,6 +71,18 @@ function Plugin.new()
 		label.Parent = screenGui
 
 		self._label = screenGui
+
+		-- If our UI was destroyed, we assume it was from another instance of
+		-- the Rojo plugin coming online.
+		--
+		-- Roblox doesn't notify plugins when they get unloaded, so this is the
+		-- best trigger we have right now unless we create a dedicated event
+		-- object.
+		screenGui.AncestryChanged:Connect(function(_, parent)
+			if parent == nil then
+				self:restart()
+			end
+		end)
 	end
 
 	return self
@@ -66,7 +95,11 @@ end
 function Plugin:restart()
 	warn("Rojo: The server has changed since the last request, reloading plugin...")
 
-	self._reconciler:clear()
+	self:stopPolling()
+
+	self._reconciler:destruct()
+	self._reconciler = Reconciler.new()
+
 	self._api = nil
 	self._polling = false
 	self._syncInProgress = false
@@ -122,7 +155,7 @@ function Plugin:stopPolling()
 		return Promise.resolve(false)
 	end
 
-	print("Rojo Stopped polling server for changes.")
+	print("Rojo: Stopped polling server for changes.")
 
 	self._polling = false
 	self._label.Enabled = false

plugin/src/Reconciler.lua

@@ -1,15 +1,32 @@
 local RouteMap = require(script.Parent.RouteMap)
 
-local function classEqual(rbx, className)
-	if className == "*" then
+local function classEqual(a, b)
+	if a == "*" or b == "*" then
 		return true
 	end
 
-	return rbx.ClassName == className
+	return a == b
 end
 
+local function applyProperties(target, properties)
+	for key, property in pairs(properties) do
+		-- TODO: Transform property value based on property.Type
+		-- Right now, we assume that 'value' is primitive!
+		target[key] = property.Value
+	end
+end
+
+--[[
+	Attempt to parent `rbx` to `parent`, doing nothing if:
+	* parent is already `parent`
+	* Changing parent threw an error
+]]
 local function reparent(rbx, parent)
 	if rbx then
+		if rbx.Parent == parent then
+			return
+		end
+
 		-- It's possible that 'rbx' is a service or some other object that we
 		-- can't change the parent of. That's the only reason why Parent would
 		-- fail except for rbx being previously destroyed!
@@ -38,7 +55,7 @@ local function findNextChildPair(primaryChildren, secondaryChildren, visited)
 			visited[primaryChild] = true
 
 			for _, secondaryChild in ipairs(secondaryChildren) do
-				if primaryChild.ClassName == secondaryChild.ClassName and primaryChild.Name == secondaryChild.Name then
+				if classEqual(primaryChild.ClassName, secondaryChild.ClassName) and primaryChild.Name == secondaryChild.Name then
 					visited[secondaryChild] = true
 
 					return primaryChild, secondaryChild
@@ -110,12 +127,11 @@ function Reconciler:_reify(item)
 	local rbx = Instance.new(className)
 	rbx.Name = item.Name
 
-	for key, property in pairs(item.Properties) do
-		-- TODO: Check for compound types, like Vector3!
-		rbx[key] = property.Value
-	end
+	applyProperties(rbx, item.Properties)
 
-	self:_reconcileChildren(rbx, item)
+	for _, child in ipairs(item.Children) do
+		reparent(self:_reify(child), rbx)
+	end
 
 	if item.Route then
 		self._routeMap:insert(item.Route, rbx)
@@ -125,10 +141,10 @@ function Reconciler:_reify(item)
 end
 
 --[[
-	Clears any state that the Reconciler has, effectively restarting it.
+	Clears any state that the Reconciler has, stopping it completely.
 ]]
-function Reconciler:clear()
-	self._routeMap:clear()
+function Reconciler:destruct()
+	self._routeMap:destruct()
 end
 
 --[[
@@ -139,6 +155,7 @@ function Reconciler:reconcile(rbx, item)
 	-- Item was deleted
 	if not item then
 		if rbx then
+			self._routeMap:removeByRbx(rbx)
 			rbx:Destroy()
 		end
 
@@ -151,27 +168,16 @@ function Reconciler:reconcile(rbx, item)
 	end
 
 	-- Item changed type!
-	if not classEqual(rbx, item.ClassName) then
+	if not classEqual(rbx.ClassName, item.ClassName) then
+		self._routeMap:removeByRbx(rbx)
 		rbx:Destroy()
 
-		rbx = self:_reify(item)
+		return self:_reify(item)
 	end
 
-	-- Apply all properties, Roblox will de-duplicate changes
-	for key, property in pairs(item.Properties) do
-		-- TODO: Transform property value based on property.Type
-		-- Right now, we assume that 'value' is primitive!
-
-		rbx[key] = property.Value
-	end
-
-	-- Use a dumb algorithm for reconciling children
+	applyProperties(rbx, item.Properties)
 	self:_reconcileChildren(rbx, item)
 
-	if item.Route then
-		self._routeMap:insert(item.Route, rbx)
-	end
-
 	return rbx
 end
 

plugin/src/RouteMap.lua

@@ -25,6 +25,10 @@ end
 function RouteMap:insert(route, rbx)
 	local hashed = hashRoute(route)
 
+	-- Make sure that each route and instance are only present in RouteMap once.
+	self:removeByRoute(route)
+	self:removeByRbx(rbx)
+
 	self._map[hashed] = rbx
 	self._reverseMap[rbx] = hashed
 	self._connectionsByRbx[rbx] = rbx.AncestryChanged:Connect(function(_, parent)
@@ -42,24 +46,36 @@ function RouteMap:removeByRoute(route)
 	local hashedRoute = hashRoute(route)
 	local rbx = self._map[hashedRoute]
 
-	if rbx then
-		self._map[hashedRoute] = nil
-		self._reverseMap[rbx] = nil
-		self._connectionsByRbx[rbx] = nil
+	if rbx ~= nil then
+		self:_removeInternal(hashedRoute, rbx)
 	end
 end
 
 function RouteMap:removeByRbx(rbx)
 	local hashedRoute = self._reverseMap[rbx]
 
-	if hashedRoute then
-		self._map[hashedRoute] = nil
-		self._reverseMap[rbx] = nil
-		self._connectionsByRbx[rbx] = nil
+	if hashedRoute ~= nil then
+		self:_removeInternal(hashedRoute, rbx)
 	end
 end
 
-function RouteMap:removeRbxDescendants(parentRbx)
+--[[
+	Correcly removes the given Roblox Instance/Route pair from the RouteMap.
+]]
+function RouteMap:_removeInternal(rbx, hashedRoute)
+	self._map[hashedRoute] = nil
+	self._reverseMap[rbx] = nil
+	self._connectionsByRbx[rbx]:Disconnect()
+	self._connectionsByRbx[rbx] = nil
+
+	self:removeRbxDescendants(rbx)
+end
+
+--[[
+	Ensure that there are no descendants of the given Roblox Instance still
+	present in the map, guaranteeing that it has been cleaned out.
+]]
+function RouteMap:_removeRbxDescendants(parentRbx)
 	for rbx in pairs(self._reverseMap) do
 		if rbx:IsDescendantOf(parentRbx) then
 			self:removeByRbx(rbx)
@@ -67,7 +83,11 @@ function RouteMap:removeRbxDescendants(parentRbx)
 	end
 end
 
-function RouteMap:clear()
+--[[
+	Remove all items from the map and disconnect all connections, cleaning up
+	the RouteMap.
+]]
+function RouteMap:destruct()
 	self._map = {}
 	self._reverseMap = {}
 

server/Cargo.lock

@@ -636,7 +636,7 @@ dependencies = [
 
 [[package]]
 name = "rojo"
-version = "0.4.5"
+version = "0.4.6"
 dependencies = [
  "clap 2.31.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "lazy_static 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",

server/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "rojo"
-version = "0.4.5"
+version = "0.4.6"
 authors = ["Lucien Greathouse <me@lpghatguy.com>"]
 description = "A tool to create robust Roblox projects"
 license = "MIT"

test-project/src/a/b.lua

@@ -0,0 +1 @@
+print("HEY!")