Release 0.4.11

Closes #66.

CHANGES.md

@@ -3,6 +3,17 @@
 ## Current master
 * *No changes*
 
+## 0.4.11 (June 10, 2018)
+* Defensively insert existing instances into RouteMap; should fix most duplication cases when syncing into existing trees.
+* Fixed incorrect synchronization from `Plugin:_pull` that would cause polling to create issues
+* Fixed incorrect file routes being assigned to `init.lua` and `init.model.json` files
+* Untangled route handling-internals slightly
+
+## 0.4.10 (June 2, 2018)
+* Added support for `init.model.json` files, which enable versioning `Tool` instances (among other things) with Rojo. ([#66](https://github.com/LPGhatguy/rojo/issues/66))
+* Fixed obscure error when syncing into an invalid service.
+* Fixed multiple sync processes occurring when a server ID mismatch is detected.
+
 ## 0.4.9 (May 26, 2018)
 * Fixed warning when renaming or removing files that would sometimes corrupt the instance cache ([#72](https://github.com/LPGhatguy/rojo/pull/72))
 * JSON models are no longer as strict -- `Children` and `Properties` are now optional.

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.9-brightgreen.svg" alt="Current server version" />
+    <img src="https://img.shields.io/badge/latest_version-0.4.11-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>

docs/getting-started/installation.md

@@ -20,4 +20,7 @@ To install the plugin, either:
 * Install the plugin from the [Roblox plugin page](https://www.roblox.com/library/1211549683/Rojo).
   * This gives you less control over what version you install -- you will always have the latest version.
 * Or, download the latest release from [the GitHub releases section](https://github.com/LPGhatguy/rojo/releases) and install it into your Roblox plugins folder
-  * You can open this folder by clicking the "Plugins Folder" button from the Plugins toolbar in Roblox Studio
+  * You can open this folder by clicking the "Plugins Folder" button from the Plugins toolbar in Roblox Studio
+
+## Visual Studio Code Extension
+If you use Visual Studio Code on Windows, you can install [Evaera's unofficial Rojo extension](https://marketplace.visualstudio.com/items?itemName=evaera.vscode-rojo), which will install both halves of Rojo for you. It even has a nifty UI to add partitions and start/stop the Rojo server!

docs/sync-details.md

@@ -30,9 +30,14 @@ Rojo supports a JSON model format for representing simple models. It's designed
 
 Rojo JSON models are stored in `.model.json` files.
 
+Starting in Rojo version **0.4.10**, model files named `init.model.json` that are located in folders will replace that folder, much like Rojo's `init.lua` support. This can be useful to version instances like `Tool` that tend to contain several instances as well as one or more scripts.
+
 !!! info
     In the future, Rojo will support `.rbxmx` models. See [issue #7](https://github.com/LPGhatguy/rojo/issues/7) for more details and updates on this feature.
 
+!!! warning
+    Prior to Rojo version **0.4.9**, the `Properties` and `Children` properties are required on all instances in JSON models!
+
 JSON model files are fairly strict; any syntax errors will cause the model to fail to sync! They look like this:
 
 `hello.model.json`

plugin/.luacheckrc

@@ -39,10 +39,6 @@ stds.testez = {
 
 ignore = {
 	"212", -- unused arguments
-	"421", -- shadowing local variable
-	"422", -- shadowing argument
-	"431", -- shadowing upvalue
-	"432", -- shadowing upvalue argument
 }
 
 std = "lua51+roblox"

plugin/src/Api.lua

@@ -35,6 +35,9 @@ function Api.connect(http)
 	setmetatable(context, Api)
 
 	return context:_start()
+		:andThen(function()
+			return context
+		end)
 end
 
 function Api:_start()
@@ -60,8 +63,6 @@ function Api:_start()
 
 			self.serverId = response.serverId
 			self.currentTime = response.currentTime
-
-			return self
 		end)
 end
 

plugin/src/Config.lua

@@ -1,6 +1,6 @@
 return {
 	pollingRate = 0.2,
-	version = {0, 4, 9},
+	version = {0, 4, 11},
 	expectedServerVersionString = "0.4.x",
 	protocolVersion = 1,
 	icons = {

plugin/src/Plugin.lua

@@ -8,6 +8,9 @@ local Api = require(script.Parent.Api)
 local Reconciler = require(script.Parent.Reconciler)
 local Version = require(script.Parent.Version)
 
+local MESSAGE_SERVER_CHANGED = "Rojo: The server has changed since the last request, reloading plugin..."
+local MESSAGE_PLUGIN_CHANGED = "Rojo: Another instance of Rojo came online, unloading..."
+
 local function collectMatch(source, pattern)
 	local result = {}
 
@@ -80,6 +83,7 @@ function Plugin.new()
 		-- object.
 		screenGui.AncestryChanged:Connect(function(_, parent)
 			if parent == nil then
+				warn(MESSAGE_PLUGIN_CHANGED)
 				self:restart()
 			end
 		end)
@@ -93,8 +97,6 @@ end
 	restarted.
 ]]
 function Plugin:restart()
-	warn("Rojo: The server has changed since the last request, reloading plugin...")
-
 	self:stopPolling()
 
 	self._reconciler:destruct()
@@ -105,22 +107,25 @@ function Plugin:restart()
 	self._syncInProgress = false
 end
 
-function Plugin:api()
+function Plugin:getApi()
-	if not self._api then
+	if self._api == nil then
-		self._api = Api.connect(self._http)
+		return Api.connect(self._http)
-			:catch(function(err)
+			:andThen(function(api)
-				self._api = nil
+				self._api = api
+
+				return api
+			end, function(err)
 				return Promise.reject(err)
 			end)
 	end
 
-	return self._api
+	return Promise.resolve(self._api)
 end
 
 function Plugin:connect()
 	print("Rojo: Testing connection...")
 
-	return self:api()
+	return self:getApi()
 		:andThen(function(api)
 			local ok, info = api:getInfo():await()
 
@@ -134,6 +139,7 @@ function Plugin:connect()
 		end)
 		:catch(function(err)
 			if err == Api.Error.ServerIdMismatch then
+				warn(MESSAGE_SERVER_CHANGED)
 				self:restart()
 				return self:connect()
 			else
@@ -163,38 +169,34 @@ function Plugin:stopPolling()
 	return Promise.resolve(true)
 end
 
-function Plugin:_pull(api, project, routes)
+function Plugin:_pull(api, project, fileRoutes)
-	return api:read(routes)
+	return api:read(fileRoutes)
 		:andThen(function(items)
-			for index = 1, #routes do
+			for index = 1, #fileRoutes do
-				local itemRoute = routes[index]
+				local fileRoute = fileRoutes[index]
-				local partitionName = itemRoute[1]
+				local partitionName = fileRoute[1]
 				local partition = project.partitions[partitionName]
 				local item = items[index]
 
-				local partitionRoute = collectMatch(partition.target, "[^.]+")
+				local partitionTargetRbxRoute = collectMatch(partition.target, "[^.]+")
 
 				-- If the item route's length was 1, we need to rename the instance to
 				-- line up with the partition's root object name.
-				--
+				if item ~= nil and #fileRoute == 1 then
-				-- This is a HACK!
+					local objectName = partition.target:match("[^.]+$")
-				if #itemRoute == 1 then
+					item.Name = objectName
-					if item then
-						local objectName = partition.target:match("[^.]+$")
-						item.Name = objectName
-					end
 				end
 
-				local fullRoute = {}
+				local itemRbxRoute = {}
-				for _, piece in ipairs(partitionRoute) do
+				for _, piece in ipairs(partitionTargetRbxRoute) do
-					table.insert(fullRoute, piece)
+					table.insert(itemRbxRoute, piece)
 				end
 
-				for i = 2, #itemRoute do
+				for i = 2, #fileRoute do
-					table.insert(fullRoute, itemRoute[i])
+					table.insert(itemRbxRoute, fileRoute[i])
 				end
 
-				self._reconciler:reconcileRoute(fullRoute, item, itemRoute)
+				self._reconciler:reconcileRoute(itemRbxRoute, item, fileRoute)
 			end
 		end)
 end
@@ -204,25 +206,25 @@ function Plugin:startPolling()
 		return
 	end
 
-	print("Rojo: Polling server for changes...")
+	print("Rojo: Starting to poll server for changes...")
 
 	self._polling = true
 	self._label.Enabled = true
 
-	return self:api()
+	return self:getApi()
 		:andThen(function(api)
-			local syncOk, result = self:syncIn():await()
-
-			if not syncOk then
-				return Promise.reject(result)
-			end
-
 			local infoOk, info = api:getInfo():await()
 
 			if not infoOk then
 				return Promise.reject(info)
 			end
 
+			local syncOk, result = self:syncIn():await()
+
+			if not syncOk then
+				return Promise.reject(result)
+			end
+
 			while self._polling do
 				local changesOk, changes = api:getChanges():await()
 
@@ -248,12 +250,12 @@ function Plugin:startPolling()
 			end
 		end)
 		:catch(function(err)
-			self:stopPolling()
-
 			if err == Api.Error.ServerIdMismatch then
+				warn(MESSAGE_SERVER_CHANGED)
 				self:restart()
 				return self:startPolling()
 			else
+				self:stopPolling()
 				return Promise.reject(err)
 			end
 		end)
@@ -269,7 +271,7 @@ function Plugin:syncIn()
 	self._syncInProgress = true
 	print("Rojo: Syncing from server...")
 
-	return self:api()
+	return self:getApi()
 		:andThen(function(api)
 			local ok, info = api:getInfo():await()
 
@@ -277,21 +279,27 @@ function Plugin:syncIn()
 				return Promise.reject(info)
 			end
 
-			local routes = {}
+			local fileRoutes = {}
 
 			for name in pairs(info.project.partitions) do
-				table.insert(routes, {name})
+				table.insert(fileRoutes, {name})
 			end
 
-			self:_pull(api, info.project, routes)
+			local pullSuccess, pullResult = self:_pull(api, info.project, fileRoutes):await()
 
 			self._syncInProgress = false
+
+			if not pullSuccess then
+				return Promise.reject(pullResult)
+			end
+
 			print("Rojo: Sync successful!")
 		end)
 		:catch(function(err)
 			self._syncInProgress = false
 
 			if err == Api.Error.ServerIdMismatch then
+				warn(MESSAGE_SERVER_CHANGED)
 				self:restart()
 				return self:syncIn()
 			else

plugin/src/Reconciler.lua

@@ -1,6 +1,9 @@
 local RouteMap = require(script.Parent.RouteMap)
 
 local function classEqual(a, b)
+	assert(typeof(a) == "string")
+	assert(typeof(b) == "string")
+
 	if a == "*" or b == "*" then
 		return true
 	end
@@ -9,6 +12,9 @@ local function classEqual(a, b)
 end
 
 local function applyProperties(target, properties)
+	assert(typeof(target) == "Instance")
+	assert(typeof(properties) == "table")
+
 	for key, property in pairs(properties) do
 		-- TODO: Transform property value based on property.Type
 		-- Right now, we assume that 'value' is primitive!
@@ -22,18 +28,19 @@ end
 	* Changing parent threw an error
 ]]
 local function reparent(rbx, parent)
-	if rbx then
+	assert(typeof(rbx) == "Instance")
-		if rbx.Parent == parent then
+	assert(typeof(parent) == "Instance")
-			return
-		end
 
-		-- It's possible that 'rbx' is a service or some other object that we
+	if rbx.Parent == parent then
-		-- can't change the parent of. That's the only reason why Parent would
+		return
-		-- fail except for rbx being previously destroyed!
-		pcall(function()
-			rbx.Parent = parent
-		end)
 	end
+
+	-- Setting `Parent` can fail if:
+	-- * The object has been destroyed
+	-- * The object is a service and cannot be reparented
+	pcall(function()
+		rbx.Parent = parent
+	end)
 end
 
 --[[
@@ -94,22 +101,30 @@ function Reconciler:_reconcileChildren(rbx, item)
 	while true do
 		local itemChild, rbxChild = findNextChildPair(item.Children, rbxChildren, visited)
 
-		if not itemChild then
+		if itemChild == nil then
 			break
 		end
 
-		reparent(self:reconcile(rbxChild, itemChild), rbx)
+		local newRbxChild = self:reconcile(rbxChild, itemChild)
+
+		if newRbxChild ~= nil then
+			newRbxChild.Parent = rbx
+		end
 	end
 
 	-- Reconcile any children that were deleted
 	while true do
 		local rbxChild, itemChild = findNextChildPair(rbxChildren, item.Children, visited)
 
-		if not rbxChild then
+		if rbxChild == nil then
 			break
 		end
 
-		reparent(self:reconcile(rbxChild, itemChild), rbx)
+		local newRbxChild = self:reconcile(rbxChild, itemChild)
+
+		if newRbxChild ~= nil then
+			newRbxChild.Parent = rbx
+		end
 	end
 end
 
@@ -133,7 +148,7 @@ function Reconciler:_reify(item)
 		reparent(self:_reify(child), rbx)
 	end
 
-	if item.Route then
+	if item.Route ~= nil then
 		self._routeMap:insert(item.Route, rbx)
 	end
 
@@ -153,8 +168,8 @@ end
 ]]
 function Reconciler:reconcile(rbx, item)
 	-- Item was deleted
-	if not item then
+	if item == nil then
-		if rbx then
+		if rbx ~= nil then
 			self._routeMap:removeByRbx(rbx)
 			rbx:Destroy()
 		end
@@ -163,7 +178,7 @@ function Reconciler:reconcile(rbx, item)
 	end
 
 	-- Item was created!
-	if not rbx then
+	if rbx == nil then
 		return self:_reify(item)
 	end
 
@@ -175,29 +190,40 @@ function Reconciler:reconcile(rbx, item)
 		return self:_reify(item)
 	end
 
+	-- It's possible that the instance we're associating with this item hasn't
+	-- been inserted into the RouteMap yet.
+	if item.Route ~= nil then
+		self._routeMap:insert(item.Route, rbx)
+	end
+
 	applyProperties(rbx, item.Properties)
 	self:_reconcileChildren(rbx, item)
 
 	return rbx
 end
 
-function Reconciler:reconcileRoute(route, item, itemRoute)
+function Reconciler:reconcileRoute(rbxRoute, item, fileRoute)
 	local parent
 	local rbx = game
 
-	for i = 1, #route do
+	for i = 1, #rbxRoute do
-		local piece = route[i]
+		local piece = rbxRoute[i]
 
 		local child = rbx:FindFirstChild(piece)
 
 		-- We should get services instead of making folders here.
-		if rbx == game and not child then
+		if rbx == game and child == nil then
-			local _
+			local success
-			_, child = pcall(game.GetService, game, piece)
+			success, child = pcall(game.GetService, game, piece)
+
+			-- That isn't a valid service!
+			if not success then
+				child = nil
+			end
 		end
 
 		-- We don't want to create a folder if we're reaching our target item!
-		if not child and i ~= #route then
+		if child == nil and i ~= #rbxRoute then
 			child = Instance.new("Folder")
 			child.Parent = rbx
 			child.Name = piece
@@ -208,8 +234,8 @@ function Reconciler:reconcileRoute(route, item, itemRoute)
 	end
 
 	-- Let's check the route map!
-	if not rbx then
+	if rbx == nil then
-		rbx = self._routeMap:get(itemRoute)
+		rbx = self._routeMap:get(fileRoute)
 	end
 
 	rbx = self:reconcile(rbx, item)

server/Cargo.lock

@@ -636,7 +636,7 @@ dependencies = [
 
 [[package]]
 name = "rojo"
-version = "0.4.9"
+version = "0.4.11"
 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.9"
+version = "0.4.11"
 authors = ["Lucien Greathouse <me@lpghatguy.com>"]
 description = "A tool to create robust Roblox projects"
 license = "MIT"

server/src/plugins/json_model_plugin.rs

@@ -10,6 +10,8 @@ lazy_static! {
     static ref JSON_MODEL_PATTERN: Regex = Regex::new(r"^(.*?)\.model\.json$").unwrap();
 }
 
+static JSON_MODEL_INIT: &'static str = "init.model.json";
+
 pub struct JsonModelPlugin;
 
 impl JsonModelPlugin {
@@ -19,7 +21,7 @@ impl JsonModelPlugin {
 }
 
 impl Plugin for JsonModelPlugin {
-    fn transform_file(&self, _plugins: &PluginChain, vfs_item: &VfsItem) -> TransformFileResult {
+    fn transform_file(&self, plugins: &PluginChain, vfs_item: &VfsItem) -> TransformFileResult {
         match vfs_item {
             &VfsItem::File { ref contents, .. } => {
                 let rbx_name = match JSON_MODEL_PATTERN.captures(vfs_item.name()) {
@@ -41,11 +43,57 @@ impl Plugin for JsonModelPlugin {
 
                 TransformFileResult::Value(Some(rbx_item))
             },
-            &VfsItem::Dir { .. } => TransformFileResult::Pass,
+            &VfsItem::Dir { ref children, .. } => {
+                let init_item = match children.get(JSON_MODEL_INIT) {
+                    Some(v) => v,
+                    None => return TransformFileResult::Pass,
+                };
+
+                let mut rbx_item = match self.transform_file(plugins, init_item) {
+                    TransformFileResult::Value(Some(item)) => item,
+                    TransformFileResult::Value(None) | TransformFileResult::Pass => {
+                        eprintln!("Inconsistency detected in JsonModelPlugin!");
+                        return TransformFileResult::Pass;
+                    },
+                };
+
+                rbx_item.name.clear();
+                rbx_item.name.push_str(vfs_item.name());
+                rbx_item.route = Some(vfs_item.route().to_vec());
+
+                for (child_name, child_item) in children {
+                    if child_name == init_item.name() {
+                        continue;
+                    }
+
+                    match plugins.transform_file(child_item) {
+                        Some(child_rbx_item) => {
+                            rbx_item.children.push(child_rbx_item);
+                        },
+                        _ => {},
+                    }
+                }
+
+                TransformFileResult::Value(Some(rbx_item))
+            },
         }
     }
 
-    fn handle_file_change(&self, _route: &Route) -> FileChangeResult {
+    fn handle_file_change(&self, route: &Route) -> FileChangeResult {
-        FileChangeResult::Pass
+        let leaf = match route.last() {
+            Some(v) => v,
+            None => return FileChangeResult::Pass,
+        };
+
+        let is_init = leaf == JSON_MODEL_INIT;
+
+        if is_init {
+            let mut changed = route.clone();
+            changed.pop();
+
+            FileChangeResult::MarkChanged(Some(vec![changed]))
+        } else {
+            FileChangeResult::Pass
+        }
     }
 }

server/src/plugins/script_plugin.rs

@@ -79,6 +79,7 @@ impl Plugin for ScriptPlugin {
 
                 rbx_item.name.clear();
                 rbx_item.name.push_str(vfs_item.name());
+                rbx_item.route = Some(vfs_item.route().to_vec());
 
                 for (child_name, child_item) in children {
                     if child_name == init_item.name() {

test-project/src/SomeTool/SomeTool.client.lua

@@ -0,0 +1 @@
+print("Hello, world, from my tool!")

test-project/src/SomeTool/init.model.json

@@ -0,0 +1,4 @@
+{
+  "Name": "SomeTool",
+  "ClassName": "Tool"
+}