Add ChangeBatcher to plugin for two-way sync (#478)

* Implement ChangeBatcher

* Use ChangeBatcher for two-way sync

* Pause updates during patch application

* I can English good

* Break after encountering a nil Parent change

This prevents __flush from erroring out when an instance's Parent is
changed to nil and it has other property changes in the same batch.

* Update rbx_dom_lua

* Don't connect changed listeners in a running game

 #468 made me realize how bad of an idea this is in general...

* Update TestEZ and fix sibling Ref reification test

* Add ChangeBatcher tests

* Test instance unpausing by breaking functionality out to __cycle

* Break up the module a bit and improve tests

* Shuffle requires around and edit comment

* Break out more stuff, rename createChangePatch -> createPatchSet

* Make ChangeBatcher responsible for unpausing all paused instances

This somewhat improves the situation (of course, it would preferrable
to not have to hack around this problem with Source at all). It also
sets us up nicely if we come across any other properties that do
anything similar.

* Remove old reference to pausedBatchInstances

* Use RenderStepped instead of Heartbeat and trash multi-frame pauses

I probably should have done this in the first place...

ChangeBatcher still needs to unpause instances, but we don't need to
hold pauses for any longer than one cycle.

* Remove useless branch

* if not next(x) -> if next(x) == nil

* Add InstanceMap:unpauseAllInstances, use it in ChangeBatcher

* Move IsRunning check to InstanceMap:__maybeFireInstanceChanged

plugin/src/ServeSession.lua

@@ -5,6 +5,7 @@ local Log = require(script.Parent.Parent.Log)
 local Fmt = require(script.Parent.Parent.Fmt)
 local t = require(script.Parent.Parent.t)
 
+local ChangeBatcher = require(script.Parent.ChangeBatcher)
 local InstanceMap = require(script.Parent.InstanceMap)
 local PatchSet = require(script.Parent.PatchSet)
 local Reconciler = require(script.Parent.Reconciler)
@@ -56,10 +57,19 @@ function ServeSession.new(options)
 	-- Declare self ahead of time to capture it in a closure
 	local self
 	local function onInstanceChanged(instance, propertyName)
-		self:__onInstanceChanged(instance, propertyName)
+		if not self.__twoWaySync then
+			return
+		end
+
+		self.__changeBatcher:add(instance, propertyName)
+	end
+
+	local function onChangesFlushed(patch)
+		self.__apiContext:write(patch)
 	end
 
 	local instanceMap = InstanceMap.new(onInstanceChanged)
+	local changeBatcher = ChangeBatcher.new(instanceMap, onChangesFlushed)
 	local reconciler = Reconciler.new(instanceMap)
 
 	local connections = {}
@@ -82,6 +92,7 @@ function ServeSession.new(options)
 		__twoWaySync = options.twoWaySync,
 		__reconciler = reconciler,
 		__instanceMap = instanceMap,
+		__changeBatcher = changeBatcher,
 		__statusChangedCallback = nil,
 		__connections = connections,
 	}
@@ -179,55 +190,6 @@ function ServeSession:__onActiveScriptChanged(activeScript)
 	self.__apiContext:open(scriptId)
 end
 
-function ServeSession:__onInstanceChanged(instance, propertyName)
-	if not self.__twoWaySync then
-		return
-	end
-
-	local instanceId = self.__instanceMap.fromInstances[instance]
-
-	if instanceId == nil then
-		Log.warn("Ignoring change for instance {:?} as it is unknown to Rojo", instance)
-		return
-	end
-
-	local remove = nil
-
-	local update = {
-		id = instanceId,
-		changedProperties = {},
-	}
-
-	if propertyName == "Name" then
-		update.changedName = instance.Name
-	elseif propertyName == "Parent" then
-		if instance.Parent == nil then
-			update = nil
-			remove = instanceId
-		else
-			Log.warn("Cannot sync non-nil Parent property changes yet")
-			return
-		end
-	else
-		local success, encoded = self.__reconciler:encodeApiValue(instance[propertyName])
-
-		if not success then
-			Log.warn("Could not sync back property {:?}.{}", instance, propertyName)
-			return
-		end
-
-		update.changedProperties[propertyName] = encoded
-	end
-
-	local patch = {
-		removed = {remove},
-		added = {},
-		updated = {update},
-	}
-
-	self.__apiContext:write(patch)
-end
-
 function ServeSession:__initialSync(rootInstanceId)
 	return self.__apiContext:read({ rootInstanceId })
 		:andThen(function(readResponseBody)
@@ -290,6 +252,7 @@ function ServeSession:__stopInternal(err)
 	self:__setStatus(Status.Disconnected, err)
 	self.__apiContext:disconnect()
 	self.__instanceMap:stop()
+	self.__changeBatcher:stop()
 
 	for _, connection in ipairs(self.__connections) do
 		connection:Disconnect()
@@ -305,4 +268,4 @@ function ServeSession:__setStatus(status, detail)
 	end
 end
 
-return ServeSession
+return ServeSession