Use GetObjects as fallback for instances that can't be created normally

Cargo.toml

@@ -81,7 +81,7 @@ regex = "1.3.1"
 reqwest = "0.9.20"
 ritz = "0.1.0"
 rlua = "0.17.0"
-roblox_install = "0.2.2"
+roblox_install = "0.3.0"
 serde = { version = "1.0", features = ["derive", "rc"] }
 serde_json = "1.0"
 structopt = "0.3.5"

plugin/src/ApiContext.lua

@@ -248,4 +248,20 @@ function ApiContext:open(id)
 		end)
 end
 
+function ApiContext:createAssets(assets)
+	local url = ("%s/api/create-assets"):format(self.__baseUrl)
+	local body = Http.jsonEncode({
+		sessionId = self.__sessionId,
+		assets = assets,
+	})
+
+	return Http.post(url, body)
+		:andThen(rejectFailedRequests)
+		:andThen(Http.Response.json)
+		:andThen(function(responseBody)
+			Log.trace("createAssets response: {:#?}", responseBody)
+			return responseBody.url
+		end)
+end
+
 return ApiContext

plugin/src/ServeSession.lua

@@ -6,8 +6,10 @@ local t = require(script.Parent.Parent.t)
 
 local InstanceMap = require(script.Parent.InstanceMap)
 local PatchSet = require(script.Parent.PatchSet)
+local Promise = require(script.Parent.Parent.Promise)
 local Reconciler = require(script.Parent.Reconciler)
 local strict = require(script.Parent.strict)
+local tryGetObjects = require(script.Parent.tryGetObjects)
 
 local Status = strict("Session.Status", {
 	NotStarted = "NotStarted",
@@ -242,27 +244,43 @@ function ServeSession:__initialSync(rootInstanceId)
 			local unappliedPatch = self.__reconciler:applyPatch(catchUpPatch)
 
 			if not PatchSet.isEmpty(unappliedPatch) then
-				Log.warn("Could not apply all changes requested by the Rojo server:\n{}",
-					PatchSet.humanSummary(self.__instanceMap, unappliedPatch))
+				return tryGetObjects(self.__instanceMap, self.__apiContext, unappliedPatch)
+					:andThen(function(finallyUnappliedPatch)
+						if not PatchSet.isEmpty(finallyUnappliedPatch) then
+							Log.warn("Could not apply all changes requested by the Rojo server:\n{}",
+								PatchSet.humanSummary(self.__instanceMap, finallyUnappliedPatch))
+						end
+					end)
 			end
 		end)
 end
 
 function ServeSession:__mainSyncLoop()
 	return self.__apiContext:retrieveMessages()
 		:andThen(function(messages)
+			local getObjectsPromises = {}
+
 			for _, message in ipairs(messages) do
 				local unappliedPatch = self.__reconciler:applyPatch(message)
 
 				if not PatchSet.isEmpty(unappliedPatch) then
-					Log.warn("Could not apply all changes requested by the Rojo server:\n{}",
-						PatchSet.humanSummary(self.__instanceMap, unappliedPatch))
+					table.insert(getObjectsPromises,
+						tryGetObjects(self.__instanceMap, self.__apiContext, unappliedPatch)
+							:andThen(function(finallyUnappliedPatch)
+								if not PatchSet.isEmpty(finallyUnappliedPatch) then
+									Log.warn("Could not apply all changes requested by the Rojo server:\n{}",
+										PatchSet.humanSummary(self.__instanceMap, finallyUnappliedPatch))
+								end
+							end)
+					)
 				end
 			end
 
-			if self.__status ~= Status.Disconnected then
-				return self:__mainSyncLoop()
-			end
+			return Promise.all(getObjectsPromises):andThen(function()
+				if self.__status ~= Status.Disconnected then
+					return self:__mainSyncLoop()
+				end
+			end)
 		end)
 end
 

plugin/src/tryGetObjects.lua

@@ -0,0 +1,106 @@
+local Log = require(script.Parent.Parent.Log)
+local PatchSet = require(script.Parent.PatchSet)
+local Promise = require(script.Parent.Parent.Promise)
+
+local function tryGetObjects(instanceMap, apiContext, patch)
+	assert(PatchSet.validate(patch))
+
+	local unappliedPatch = PatchSet.newEmpty()
+
+	-- GetObjects won't help with anything that failed to remove
+	unappliedPatch.removed = patch.removed
+
+	local assetsToRequest = {}
+	local receiveCallbacks = {}
+
+	Log.trace("tryGetObjects({:#?})", patch)
+
+	for id, addition in pairs(patch.added) do
+		unappliedPatch.added[id] = addition
+
+		table.insert(assetsToRequest, id)
+		table.insert(receiveCallbacks, function(newInstance)
+			for _, childId in ipairs(addition.Children) do
+				local child = instanceMap.fromIds[childId]
+				if child == nil then
+					Log.warn("Got child ID that wasn't in the instance map: {}", childId)
+					continue
+				end
+
+				child.Parent = newInstance
+			end
+
+			if addition.Parent ~= nil then
+				local parent = instanceMap.fromIds[addition.Parent]
+				if parent == nil then
+					Log.warn("Instance tried to be parented to non-existent parent: {}", addition.Parent)
+					return
+				end
+
+				local ok, problem = pcall(function()
+					newInstance.Parent = parent
+				end)
+
+				if not ok then
+					Log.warn("GetObjects couldn't parent {} to {}: {}", newInstance, parent, problem)
+					return
+				end
+			end
+
+			unappliedPatch.added[id] = nil
+			instanceMap:insert(id, newInstance)
+		end)
+	end
+
+	-- GetObjects only create instances, we can't update the properties of existing ones.
+	-- Instead, just create them again, move their children, and replace the instance.
+	for _, update in ipairs(patch.updated) do
+		-- If no properties were changed during an update, GetObjects isn't going to do anything that hasn't already been tried
+		if next(update.changedProperties) == nil then
+			continue
+		end
+
+		table.insert(assetsToRequest, update.id)
+		table.insert(unappliedPatch.updated, update)
+
+		table.insert(receiveCallbacks, function(newInstance)
+			local oldInstance = instanceMap.fromIds[update.id]
+
+			for _, oldChild in ipairs(oldInstance:GetChildren()) do
+				oldChild.Parent = newInstance
+			end
+
+			local oldParent = oldInstance.Parent
+			instanceMap:destroyInstance(oldInstance)
+
+			local ok = pcall(function()
+				newInstance.Parent = oldParent
+			end)
+
+			if ok then
+				table.remove(unappliedPatch.updated, table.find(unappliedPatch.updated, update))
+				instanceMap:insert(update.id, newInstance)
+			end
+		end)
+	end
+
+	Log.trace("assetsToRequest = {:?}", assetsToRequest)
+
+	if #assetsToRequest == 0 then
+		return Promise.resolve(unappliedPatch)
+	end
+
+	return apiContext:createAssets(assetsToRequest):andThen(function(assetId)
+		-- GetObjects doesn't yield, it blocks.
+		-- There is no way to promisify this unless GetObjectsAsync is opened up.
+		local createdAssets = game:GetObjects(assetId)
+
+		for index, assetInstance in ipairs(createdAssets) do
+			receiveCallbacks[index](assetInstance)
+		end
+
+		return unappliedPatch
+	end)
+end
+
+return tryGetObjects

plugin/src/tryGetObjects.spec.lua

@@ -0,0 +1,89 @@
+return function()
+	local InstanceMap = require(script.Parent.InstanceMap)
+	local PatchSet = require(script.Parent.PatchSet)
+	local Promise = require(script.Parent.Parent.Promise)
+	local tryGetObjects = require(script.Parent.tryGetObjects)
+
+	-- game:GetObjects(ASSET_DOGE) == { Doge mesh }
+	local ASSET_DOGE = "rbxassetid://4574885387"
+
+	-- This is the actual Mesh ID for Doge
+	local MESH_DOGE = "rbxassetid://4574885352"
+
+	local function createMockApiContext()
+		local mockApiContext = {}
+		function mockApiContext:createAssets()
+			return Promise.resolve(ASSET_DOGE)
+		end
+
+		return mockApiContext
+	end
+
+	it("should apply updates to existing instances", function()
+		local instanceMap = InstanceMap.new()
+
+		local oldDoge = Instance.new("MeshPart")
+		instanceMap:insert("DOGE", oldDoge)
+
+		local patch = PatchSet.newEmpty()
+		table.insert(patch.updated, {
+			id = "DOGE",
+			changedProperties = {
+				MeshId = {
+					Type = "Content",
+					Value = MESH_DOGE,
+				},
+			},
+		})
+
+		local _, unappliedPatch = assert(
+			tryGetObjects(instanceMap, createMockApiContext(), patch)
+				:await()
+		)
+
+		assert(PatchSet.isEmpty(unappliedPatch), "expected remaining patch to be empty")
+
+		local newDoge = instanceMap.fromIds["DOGE"]
+		assert(newDoge ~= nil, "no instance with id DOGE")
+		expect(newDoge.MeshId).to.equal(MESH_DOGE)
+	end)
+
+	it("should be able to create new instances", function()
+		local instanceMap = InstanceMap.new()
+
+		local folder = Instance.new("Folder")
+		instanceMap:insert("PARENT", folder)
+
+		local child = Instance.new("Folder")
+		child.Name = "Child"
+		instanceMap:insert("CHILD", child)
+
+		local patch = PatchSet.newEmpty()
+
+		patch.added["DOGE"] = {
+			Id = "DOGE",
+			Name = "Doge",
+			ClassName = "MeshPart",
+			Parent = "PARENT",
+			Properties = {
+				MeshId = {
+					Type = "Content",
+					Value = MESH_DOGE,
+				},
+			},
+			Children = { "CHILD" },
+		}
+
+		local _, unappliedPatch = assert(
+			tryGetObjects(instanceMap, createMockApiContext(), patch)
+				:await()
+		)
+
+		assert(PatchSet.isEmpty(unappliedPatch), "expected remaining patch to be empty")
+
+		local doge = instanceMap.fromIds["DOGE"]
+		assert(doge ~= nil, "no instance with id DOGE")
+		expect(doge.Parent).to.equal(folder)
+		expect(doge:FindFirstChild("Child")).to.equal(child)
+	end)
+end

plugin/test-place.project.json

@@ -9,7 +9,7 @@
       },
 
       "TestEZ": {
-        "$path": "modules/testez/lib"
+        "$path": "modules/testez/src"
       }
     },
 

plugin/test.ps1

@@ -0,0 +1,2 @@
+rojo build test-place.project.json -o TestPlace.rbxlx
+run-in-roblox --script run-tests.server.lua --place TestPlace.rbxlx

selene.toml

@@ -1,4 +1,4 @@
-std = "roblox"
+std = "roblox+testez"
 
 [config]
 unused_variable = { allow_unused_self = true }