diff --git a/CHANGES.md b/CHANGES.md index a4b8ed62..0e6a1c95 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,6 +2,7 @@ ## Current Master * Plugin now automatically selects `HttpService` if it determines that HTTP isn't enabled ([#58](https://github.com/LPGhatguy/rojo/pull/58)) +* Plugin now has much more robust handling and will wipe all state when the server changes. ## 0.4.2 (April 4, 2018) * Fixed final case of duplicated instance insertion, caused by reconciled instances not being inserted into `RouteMap`. diff --git a/plugin/src/Server.lua b/plugin/src/Api.lua similarity index 66% rename from plugin/src/Server.lua rename to plugin/src/Api.lua index 5619426f..131acf6f 100644 --- a/plugin/src/Server.lua +++ b/plugin/src/Api.lua @@ -4,32 +4,43 @@ local Config = require(script.Parent.Config) local Promise = require(script.Parent.Promise) local Version = require(script.Parent.Version) -local Server = {} -Server.__index = Server +local Api = {} +Api.__index = Api + +Api.Error = { + ServerIdMismatch = "ServerIdMismatch", +} + +setmetatable(Api.Error, { + __index = function(_, key) + error("Invalid API.Error name " .. key, 2) + end +}) --[[ - Create a new Server using the given HTTP implementation and replacer. + Api.connect(Http) -> Promise - If the context becomes invalid, `replacer` will be invoked with a new - context that should be suitable to replace this one. + Create a new Api using the given HTTP implementation. Attempting to invoke methods on an invalid conext will throw errors! ]] -function Server.connect(http) +function Api.connect(http) local context = { http = http, serverId = nil, currentTime = 0, } - setmetatable(context, Server) + setmetatable(context, Api) return context:_start() end -function Server:_start() - return self:getInfo() +function Api:_start() + return self.http:get("/") :andThen(function(response) + response = response:json() + if response.protocolVersion ~= Config.protocolVersion then local message = ( "Found a Rojo dev server, but it's using a different protocol version, and is incompatible." .. @@ -39,7 +50,7 @@ function Server:_start() "\n\nGo to https://github.com/LPGhatguy/rojo for more details." ):format( Version.display(Config.version), Config.protocolVersion, - Config.expectedServerVersionString, + Config.expectedApiVersionString, response.serverVersion, response.protocolVersion ) @@ -53,37 +64,49 @@ function Server:_start() end) end -function Server:getInfo() +function Api:getInfo() return self.http:get("/") :andThen(function(response) response = response:json() + if response.serverId ~= self.serverId then + return Promise.reject(Api.Error.ServerIdMismatch) + end + return response end) end -function Server:read(paths) +function Api:read(paths) local body = HttpService:JSONEncode(paths) return self.http:post("/read", body) :andThen(function(response) response = response:json() + if response.serverId ~= self.serverId then + return Promise.reject(Api.Error.ServerIdMismatch) + end + return response.items end) end -function Server:getChanges() +function Api:getChanges() local url = ("/changes/%f"):format(self.currentTime) return self.http:get(url) :andThen(function(response) response = response:json() + if response.serverId ~= self.serverId then + return Promise.reject(Api.Error.ServerIdMismatch) + end + self.currentTime = response.currentTime return response.changes end) end -return Server +return Api diff --git a/plugin/src/Main.server.lua b/plugin/src/Main.server.lua index ca771f5b..c6570229 100644 --- a/plugin/src/Main.server.lua +++ b/plugin/src/Main.server.lua @@ -11,6 +11,11 @@ local Version = require(script.Parent.Version) are, show them a reminder to make sure they check their server version. ]] local function checkUpgrade() + -- When developing Rojo, there's no use in doing version checks + if Config.dev then + return + end + local lastVersion = plugin:GetSetting("LastRojoVersion") if lastVersion then @@ -30,10 +35,7 @@ local function checkUpgrade() end end - -- When developing Rojo, there's no use in storing that version number. - if not Config.dev then - plugin:SetSetting("LastRojoVersion", Config.version) - end + plugin:SetSetting("LastRojoVersion", Config.version) end local function main() diff --git a/plugin/src/Plugin.lua b/plugin/src/Plugin.lua index 92cdbd4b..1227c43d 100644 --- a/plugin/src/Plugin.lua +++ b/plugin/src/Plugin.lua @@ -1,6 +1,6 @@ local Config = require(script.Parent.Config) local Http = require(script.Parent.Http) -local Server = require(script.Parent.Server) +local Api = require(script.Parent.Api) local Promise = require(script.Parent.Promise) local Reconciler = require(script.Parent.Reconciler) @@ -26,7 +26,7 @@ function Plugin.new() local self = { _http = Http.new(remote), _reconciler = Reconciler.new(), - _server = nil, + _api = nil, _polling = false, } @@ -57,37 +57,58 @@ function Plugin.new() return self end -function Plugin:server() - if not self._server then - self._server = Server.connect(self._http) +--[[ + Clears all state and issues a notice to the user that the plugin has + restarted. +]] +function Plugin:restart() + warn("The server has changed since the last request, reloading plugin...") + + self._reconciler:clear() + self._api = nil + self._polling = false +end + +function Plugin:api() + if not self._api then + self._api = Api.connect(self._http) :catch(function(err) - self._server = nil + self._api = nil return Promise.reject(err) end) end - return self._server + return self._api end function Plugin:connect() print("Testing connection...") - return self:server() - :andThen(function(server) - return server:getInfo() - end) - :andThen(function(result) + return self:api() + :andThen(function(api) + local ok, info = api:getInfo():await() + + if not ok then + return Promise.reject(info) + end + print("Server found!") - print("Protocol version:", result.protocolVersion) - print("Server version:", result.serverVersion) + print("Protocol version:", info.protocolVersion) + print("Server version:", info.serverVersion) + end) + :catch(function(err) + if err == Api.Error.ServerIdMismatch then + self:restart() + return self:connect() + else + return Promise.reject(err) + end end) end function Plugin:togglePolling() if self._polling then - self:stopPolling() - - return Promise.resolve(nil) + return self:stopPolling() else return self:startPolling() end @@ -95,48 +116,51 @@ end function Plugin:stopPolling() if not self._polling then - return + return Promise.resolve(false) end print("Stopped polling.") self._polling = false self._label.Enabled = false + + return Promise.resolve(true) end -function Plugin:_pull(server, project, routes) - local items = server:read(routes):await() +function Plugin:_pull(api, project, routes) + return api:read(routes) + :andThen(function(items) + for index = 1, #routes do + local itemRoute = routes[index] + local partitionName = itemRoute[1] + local partition = project.partitions[partitionName] + local item = items[index] - for index = 1, #routes do - local itemRoute = routes[index] - local partitionName = itemRoute[1] - local partition = project.partitions[partitionName] - local item = items[index] + local partitionRoute = collectMatch(partition.target, "[^.]+") - local partitionRoute = 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. + -- + -- This is a HACK! + if #itemRoute == 1 then + if item then + local objectName = partition.target:match("[^.]+$") + item.Name = objectName + end + end - -- If the item route's length was 1, we need to rename the instance to - -- line up with the partition's root object name. - -- - -- This is a HACK! - if #itemRoute == 1 then - if item then - local objectName = partition.target:match("[^.]+$") - item.Name = objectName + local fullRoute = {} + for _, piece in ipairs(partitionRoute) do + table.insert(fullRoute, piece) + end + + for i = 2, #itemRoute do + table.insert(fullRoute, itemRoute[i]) + end + + self._reconciler:reconcileRoute(fullRoute, item, itemRoute) end - end - - local fullRoute = {} - for _, piece in ipairs(partitionRoute) do - table.insert(fullRoute, piece) - end - - for i = 2, #itemRoute do - table.insert(fullRoute, itemRoute[i]) - end - - self._reconciler:reconcileRoute(fullRoute, item, itemRoute) - end + end) end function Plugin:startPolling() @@ -149,14 +173,22 @@ function Plugin:startPolling() self._polling = true self._label.Enabled = true - return self:server() - :andThen(function(server) - self:syncIn():await() + return self:api() + :andThen(function(api) + local syncOk, result = self:syncIn():await() - local project = server:getInfo():await().project + if not syncOk then + return Promise.reject(result) + end + + local infoOk, info = api:getInfo():await() + + if not infoOk then + return Promise.reject(info) + end while self._polling do - local changes = server:getChanges():await() + local changes = api:getChanges():await() if #changes > 0 then local routes = {} @@ -165,34 +197,57 @@ function Plugin:startPolling() table.insert(routes, change.route) end - self:_pull(server, project, routes) + local pullOk, pullResult = self:_pull(api, info.project, routes):await() + + if not pullOk then + return Promise.reject(pullResult) + end end wait(Config.pollingRate) end end) - :catch(function() + :catch(function(err) self:stopPolling() + + if err == Api.Error.ServerIdMismatch then + self:restart() + return self:startPolling() + else + return Promise.reject(err) + end end) end function Plugin:syncIn() print("Syncing from server...") - return self:server() - :andThen(function(server) - local project = server:getInfo():await().project + return self:api() + :andThen(function(api) + local ok, info = api:getInfo():await() + + if not ok then + return Promise.reject(info) + end local routes = {} - for name in pairs(project.partitions) do + for name in pairs(info.project.partitions) do table.insert(routes, {name}) end - self:_pull(server, project, routes) + self:_pull(api, info.project, routes) print("Sync successful!") end) + :catch(function(err) + if err == Api.Error.ServerIdMismatch then + self:restart() + return self:syncIn() + else + return Promise.reject(err) + end + end) end return Plugin diff --git a/plugin/src/Promise.lua b/plugin/src/Promise.lua index cdcea791..144469e6 100644 --- a/plugin/src/Promise.lua +++ b/plugin/src/Promise.lua @@ -221,15 +221,11 @@ function Promise:await() local ok = bindable.Event:Wait() bindable:Destroy() - if not ok then - error(tostring(result[1]), 2) - end - - return unpack(result) + return ok, unpack(result) elseif self._status == Promise.Status.Resolved then - return unpack(self._value) + return true, unpack(self._value) elseif self._status == Promise.Status.Rejected then - error(tostring(self._value[1]), 2) + return false, unpack(self._value) end end diff --git a/plugin/src/Reconciler.lua b/plugin/src/Reconciler.lua index 33f1119f..b69929fa 100644 --- a/plugin/src/Reconciler.lua +++ b/plugin/src/Reconciler.lua @@ -124,6 +124,13 @@ function Reconciler:_reify(item) return rbx end +--[[ + Clears any state that the Reconciler has, effectively restarting it. +]] +function Reconciler:clear() + self._routeMap:clear() +end + --[[ Apply the changes represented by the given item to a Roblox object that's a child of the given instance. diff --git a/plugin/src/RouteMap.lua b/plugin/src/RouteMap.lua index d76e5a0b..a707f7cd 100644 --- a/plugin/src/RouteMap.lua +++ b/plugin/src/RouteMap.lua @@ -71,8 +71,8 @@ function RouteMap:clear() self._map = {} self._reverseMap = {} - for object in pairs(self._connectionsByRbx) do - object:Disconnect() + for _, connection in pairs(self._connectionsByRbx) do + connection:Disconnect() end self._connectionsByRbx = {}