diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c090788..01d8468e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ Making a new release? Simply add the new header with the version and date undern ## Unreleased * `inf` and `nan` values in properties are now synced ([#1176]) +* Implemented the "Always" confirmation behavior setting, which prompts for confirmation on every sync patch, not just the initial sync. Changes that arrive during confirmation are merged into the pending patch and the UI updates in real-time. ([#1216]) * Fixed a bug caused by having reference properties (such as `ObjectValue.Value`) that point to an Instance not included in syncback. ([#1179]) * Fixed instance replacement fallback failing when too many instances needed to be replaced. ([#1192]) * Added actors and bindable/remote event/function variants to be synced back as JSON files. ([#1199]) @@ -47,6 +48,7 @@ Making a new release? Simply add the new header with the version and date undern [#1201]: https://github.com/rojo-rbx/rojo/pull/1201 [#1211]: https://github.com/rojo-rbx/rojo/pull/1211 [#1215]: https://github.com/rojo-rbx/rojo/pull/1215 +[#1216]: https://github.com/rojo-rbx/rojo/pull/1216 [#1217]: https://github.com/rojo-rbx/rojo/pull/1217 ## [7.7.0-rc.1] (November 27th, 2025) diff --git a/plugin/src/App/Components/PatchVisualizer/DomLabel.lua b/plugin/src/App/Components/PatchVisualizer/DomLabel.lua index bc9b110e..dc3a7c3c 100644 --- a/plugin/src/App/Components/PatchVisualizer/DomLabel.lua +++ b/plugin/src/App/Components/PatchVisualizer/DomLabel.lua @@ -72,6 +72,11 @@ function DomLabel:init() end) end +function DomLabel:willUnmount() + -- Stop the motor to prevent onStep callbacks after unmount + self.motor:stop() +end + function DomLabel:didUpdate(prevProps) if prevProps.instance ~= self.props.instance diff --git a/plugin/src/App/init.lua b/plugin/src/App/init.lua index e6e4deee..6e47ee76 100644 --- a/plugin/src/App/init.lua +++ b/plugin/src/App/init.lua @@ -751,36 +751,38 @@ function App:startSession() end local confirmationBehavior = Settings:get("confirmationBehavior") - if confirmationBehavior == "Initial" then - -- Only confirm if we haven't synced this project yet this session - if self.knownProjects[serverInfo.projectName] then - Log.trace( - "Accepting patch without confirmation because project has already been connected and behavior is set to Initial" - ) - return "Accept" - end - elseif confirmationBehavior == "Large Changes" then - -- Only confirm if the patch impacts many instances - if PatchSet.countInstances(patch) < Settings:get("largeChangesConfirmationThreshold") then - Log.trace( - "Accepting patch without confirmation because patch is small and behavior is set to Large Changes" - ) - return "Accept" - end - elseif confirmationBehavior == "Unlisted PlaceId" then - -- Only confirm if the current placeId is not in the servePlaceIds allowlist - if serverInfo.expectedPlaceIds then - local isListed = table.find(serverInfo.expectedPlaceIds, game.PlaceId) ~= nil - if isListed then + if confirmationBehavior ~= "Always" then + if confirmationBehavior == "Initial" then + -- Only confirm if we haven't synced this project yet this session + if self.knownProjects[serverInfo.projectName] then Log.trace( - "Accepting patch without confirmation because placeId is listed and behavior is set to Unlisted PlaceId" + "Accepting patch without confirmation because project has already been connected and behavior is set to Initial" ) return "Accept" end + elseif confirmationBehavior == "Large Changes" then + -- Only confirm if the patch impacts many instances + if PatchSet.countInstances(patch) < Settings:get("largeChangesConfirmationThreshold") then + Log.trace( + "Accepting patch without confirmation because patch is small and behavior is set to Large Changes" + ) + return "Accept" + end + elseif confirmationBehavior == "Unlisted PlaceId" then + -- Only confirm if the current placeId is not in the servePlaceIds allowlist + if serverInfo.expectedPlaceIds then + local isListed = table.find(serverInfo.expectedPlaceIds, game.PlaceId) ~= nil + if isListed then + Log.trace( + "Accepting patch without confirmation because placeId is listed and behavior is set to Unlisted PlaceId" + ) + return "Accept" + end + end + elseif confirmationBehavior == "Never" then + Log.trace("Accepting patch without confirmation because behavior is set to Never") + return "Accept" end - elseif confirmationBehavior == "Never" then - Log.trace("Accepting patch without confirmation because behavior is set to Never") - return "Accept" end -- The datamodel name gets overwritten by Studio, making confirmation of it intrusive @@ -821,7 +823,36 @@ function App:startSession() timeout = 7, }) - return self.confirmationEvent:Wait() + local result = self.confirmationEvent:Wait() + + -- Reset UI state back to Connected after confirmation + -- This is needed for ongoing WebSocket patches where the session + -- is already connected and won't trigger a status change + if self.serveSession and self.serveSession:getStatus() == ServeSession.Status.Connected then + self:setState({ + appStatus = AppStatus.Connected, + toolbarIcon = Assets.Images.PluginButtonConnected, + -- Clear patchTree to avoid animation issues when the + -- PatchVisualizer unmounts while Flipper motors are running + patchTree = nil, + }) + end + + return result + end) + + serveSession:setPatchUpdateCallback(function(instanceMap, patch) + -- If all changes have been reverted, auto-accept the empty patch + if PatchSet.isEmpty(patch) then + Log.trace("Patch became empty after merging, auto-accepting") + self.confirmationBindable:Fire("Accept") + return + end + + -- Update the patchTree when new changes arrive during confirmation + self:setState({ + patchTree = PatchTree.build(patch, instanceMap, { "Property", "Current", "Incoming" }), + }) end) serveSession:start() diff --git a/plugin/src/PatchSet.lua b/plugin/src/PatchSet.lua index 3f6ece3c..281f147a 100644 --- a/plugin/src/PatchSet.lua +++ b/plugin/src/PatchSet.lua @@ -7,6 +7,7 @@ local Packages = script.Parent.Parent.Packages local t = require(Packages.t) local Types = require(script.Parent.Types) +local decodeValue = require(script.Parent.Reconciler.decodeValue) local function deepEqual(a: any, b: any): boolean local typeA = typeof(a) @@ -282,6 +283,101 @@ function PatchSet.assign(target, ...) return target end +--[[ + Merge a source PatchSet into the target, properly reconciling conflicts. + Unlike assign(), this handles cases where a removal cancels out an addition, + or where an update supersedes a previous update to the same instance. + + If instanceMap is provided, updates that revert to the current instance state + will be removed from the patch entirely. +]] +function PatchSet.merge(target, source, instanceMap) + -- Process removals: if we're removing something that was added, just delete the addition + for _, removed in ipairs(source.removed) do + if target.added[removed] ~= nil then + -- This removal cancels out a previous addition + target.added[removed] = nil + else + table.insert(target.removed, removed) + end + end + + -- Process additions + for id, added in pairs(source.added) do + target.added[id] = added + end + + -- Process updates: merge with existing updates to the same instance + for _, update in ipairs(source.updated) do + local existingIndex = nil + for i, existing in ipairs(target.updated) do + if existing.id == update.id then + existingIndex = i + break + end + end + + if existingIndex then + -- Merge the updates + local existing = target.updated[existingIndex] + if update.changedName ~= nil then + existing.changedName = update.changedName + end + if update.changedClassName ~= nil then + existing.changedClassName = update.changedClassName + end + if update.changedMetadata ~= nil then + existing.changedMetadata = update.changedMetadata + end + for prop, value in pairs(update.changedProperties) do + existing.changedProperties[prop] = value + end + + -- Check if this update now matches current instance state (i.e., reverted) + -- If so, remove properties that match or remove the update entirely + if instanceMap then + local instance = instanceMap.fromIds[existing.id] + if instance then + -- Check if name change matches current + if existing.changedName ~= nil and existing.changedName == instance.Name then + existing.changedName = nil + end + -- Check if class change matches current + if existing.changedClassName ~= nil and existing.changedClassName == instance.ClassName then + existing.changedClassName = nil + end + -- Check each changed property against current instance value + for propName, encodedValue in pairs(existing.changedProperties) do + local decodeSuccess, decodedValue = decodeValue(encodedValue, instanceMap) + if decodeSuccess then + local currentSuccess, currentValue = pcall(function() + return instance[propName] + end) + if currentSuccess and deepEqual(decodedValue, currentValue) then + existing.changedProperties[propName] = nil + end + end + end + + -- If no changes remain, remove the update entirely + local hasChanges = existing.changedName ~= nil + or existing.changedClassName ~= nil + or existing.changedMetadata ~= nil + or next(existing.changedProperties) ~= nil + + if not hasChanges then + table.remove(target.updated, existingIndex) + end + end + end + else + table.insert(target.updated, update) + end + end + + return target +end + function PatchSet.addedIdList(patchSet): { string } local idList = table.create(#patchSet.added) for id in patchSet.added do diff --git a/plugin/src/PatchSet.spec.lua b/plugin/src/PatchSet.spec.lua new file mode 100644 index 00000000..df17ce4c --- /dev/null +++ b/plugin/src/PatchSet.spec.lua @@ -0,0 +1,341 @@ +return function() + local PatchSet = require(script.Parent.PatchSet) + local InstanceMap = require(script.Parent.InstanceMap) + + describe("newEmpty", function() + it("should create an empty patch", function() + local patch = PatchSet.newEmpty() + expect(PatchSet.isEmpty(patch)).to.equal(true) + end) + end) + + describe("isEmpty", function() + it("should return true for empty patches", function() + local patch = PatchSet.newEmpty() + expect(PatchSet.isEmpty(patch)).to.equal(true) + end) + + it("should return false when patch has removals", function() + local patch = PatchSet.newEmpty() + table.insert(patch.removed, "some-id") + expect(PatchSet.isEmpty(patch)).to.equal(false) + end) + + it("should return false when patch has additions", function() + local patch = PatchSet.newEmpty() + patch.added["some-id"] = { Id = "some-id", ClassName = "Folder", Name = "Test" } + expect(PatchSet.isEmpty(patch)).to.equal(false) + end) + + it("should return false when patch has updates", function() + local patch = PatchSet.newEmpty() + table.insert(patch.updated, { id = "some-id", changedProperties = {} }) + expect(PatchSet.isEmpty(patch)).to.equal(false) + end) + end) + + describe("merge", function() + it("should merge additions from source into target", function() + local target = PatchSet.newEmpty() + local source = PatchSet.newEmpty() + + source.added["CHILD"] = { + Id = "CHILD", + ClassName = "Folder", + Name = "Child", + Parent = "ROOT", + Children = {}, + Properties = {}, + } + + PatchSet.merge(target, source) + + expect(target.added["CHILD"]).to.be.ok() + expect(target.added["CHILD"].Name).to.equal("Child") + end) + + it("should merge removals from source into target", function() + local target = PatchSet.newEmpty() + local source = PatchSet.newEmpty() + + table.insert(source.removed, "CHILD") + + PatchSet.merge(target, source) + + expect(#target.removed).to.equal(1) + expect(target.removed[1]).to.equal("CHILD") + end) + + it("should cancel additions when source removes them", function() + local target = PatchSet.newEmpty() + target.added["CHILD"] = { + Id = "CHILD", + ClassName = "Folder", + Name = "Child", + Parent = "ROOT", + Children = {}, + Properties = {}, + } + + local source = PatchSet.newEmpty() + table.insert(source.removed, "CHILD") + + PatchSet.merge(target, source) + + -- The addition should be cancelled + expect(target.added["CHILD"]).to.equal(nil) + -- And no removal should be added since it was never actually applied + expect(#target.removed).to.equal(0) + end) + + it("should merge updates from source into target", function() + local target = PatchSet.newEmpty() + local source = PatchSet.newEmpty() + + table.insert(source.updated, { + id = "INSTANCE", + changedName = "NewName", + changedProperties = {}, + }) + + PatchSet.merge(target, source) + + expect(#target.updated).to.equal(1) + expect(target.updated[1].changedName).to.equal("NewName") + end) + + it("should combine updates for the same instance", function() + local target = PatchSet.newEmpty() + table.insert(target.updated, { + id = "INSTANCE", + changedName = "FirstName", + changedProperties = { + Value = { String = "First" }, + }, + }) + + local source = PatchSet.newEmpty() + table.insert(source.updated, { + id = "INSTANCE", + changedName = "SecondName", + changedProperties = { + OtherValue = { String = "Second" }, + }, + }) + + PatchSet.merge(target, source) + + -- Should still only have one update entry + expect(#target.updated).to.equal(1) + -- Name should be overwritten by the newer update + expect(target.updated[1].changedName).to.equal("SecondName") + -- Both properties should be present + expect(target.updated[1].changedProperties.Value).to.be.ok() + expect(target.updated[1].changedProperties.OtherValue).to.be.ok() + end) + + it("should remove update when reverted to current instance state", function() + -- Create an instance to compare against + local testInstance = Instance.new("StringValue") + testInstance.Name = "OriginalName" + testInstance.Value = "OriginalValue" + + local instanceMap = InstanceMap.new() + instanceMap:insert("INSTANCE", testInstance) + + -- Target has an update changing the name + local target = PatchSet.newEmpty() + table.insert(target.updated, { + id = "INSTANCE", + changedName = "ChangedName", + changedProperties = {}, + }) + + -- Source reverts the name back to original + local source = PatchSet.newEmpty() + table.insert(source.updated, { + id = "INSTANCE", + changedName = "OriginalName", + changedProperties = {}, + }) + + PatchSet.merge(target, source, instanceMap) + + -- The update should be removed entirely since it matches current state + expect(#target.updated).to.equal(0) + + testInstance:Destroy() + instanceMap:stop() + end) + + it("should remove individual property changes when reverted", function() + -- Create an instance to compare against + local testInstance = Instance.new("StringValue") + testInstance.Name = "Test" + testInstance.Value = "OriginalValue" + + local instanceMap = InstanceMap.new() + instanceMap:insert("INSTANCE", testInstance) + + -- Target has property change + local target = PatchSet.newEmpty() + table.insert(target.updated, { + id = "INSTANCE", + changedName = "NewName", + changedProperties = { + Value = { String = "ChangedValue" }, + }, + }) + + -- Source reverts the property back to original + local source = PatchSet.newEmpty() + table.insert(source.updated, { + id = "INSTANCE", + changedProperties = { + Value = { String = "OriginalValue" }, + }, + }) + + PatchSet.merge(target, source, instanceMap) + + -- Should still have update for the name change + expect(#target.updated).to.equal(1) + expect(target.updated[1].changedName).to.equal("NewName") + -- But the Value property should be removed since it matches current + expect(target.updated[1].changedProperties.Value).to.equal(nil) + + testInstance:Destroy() + instanceMap:stop() + end) + + it("should keep updates for different instances separate", function() + local target = PatchSet.newEmpty() + table.insert(target.updated, { + id = "INSTANCE_A", + changedName = "NameA", + changedProperties = {}, + }) + + local source = PatchSet.newEmpty() + table.insert(source.updated, { + id = "INSTANCE_B", + changedName = "NameB", + changedProperties = {}, + }) + + PatchSet.merge(target, source) + + expect(#target.updated).to.equal(2) + end) + end) + + describe("assign", function() + it("should merge multiple patches additively", function() + local target = PatchSet.newEmpty() + + local source1 = PatchSet.newEmpty() + source1.added["A"] = { Id = "A", ClassName = "Folder", Name = "A" } + + local source2 = PatchSet.newEmpty() + source2.added["B"] = { Id = "B", ClassName = "Folder", Name = "B" } + + PatchSet.assign(target, source1, source2) + + expect(target.added["A"]).to.be.ok() + expect(target.added["B"]).to.be.ok() + end) + end) + + describe("countChanges", function() + it("should count property changes in additions", function() + local patch = PatchSet.newEmpty() + patch.added["A"] = { + Id = "A", + ClassName = "StringValue", + Name = "A", + Properties = { + Value = { String = "test" }, + MaxValue = { Float64 = 100 }, + }, + } + + expect(PatchSet.countChanges(patch)).to.equal(2) + end) + + it("should count removals as single changes", function() + local patch = PatchSet.newEmpty() + table.insert(patch.removed, "A") + table.insert(patch.removed, "B") + + expect(PatchSet.countChanges(patch)).to.equal(2) + end) + + it("should count property updates", function() + local patch = PatchSet.newEmpty() + table.insert(patch.updated, { + id = "A", + changedProperties = { + Value = { String = "test" }, + }, + }) + + expect(PatchSet.countChanges(patch)).to.equal(1) + end) + + it("should count name changes", function() + local patch = PatchSet.newEmpty() + table.insert(patch.updated, { + id = "A", + changedName = "NewName", + changedProperties = {}, + }) + + expect(PatchSet.countChanges(patch)).to.equal(1) + end) + + it("should count className changes", function() + local patch = PatchSet.newEmpty() + table.insert(patch.updated, { + id = "A", + changedClassName = "Model", + changedProperties = {}, + }) + + expect(PatchSet.countChanges(patch)).to.equal(1) + end) + end) + + describe("countInstances", function() + it("should count all affected instances", function() + local patch = PatchSet.newEmpty() + patch.added["A"] = { Id = "A", ClassName = "Folder", Name = "A" } + patch.added["B"] = { Id = "B", ClassName = "Folder", Name = "B" } + table.insert(patch.removed, "C") + table.insert(patch.updated, { id = "D", changedProperties = {} }) + + expect(PatchSet.countInstances(patch)).to.equal(4) + end) + end) + + describe("isEqual", function() + it("should return true for identical patches", function() + local patchA = PatchSet.newEmpty() + patchA.added["A"] = { Id = "A", ClassName = "Folder", Name = "A" } + + local patchB = PatchSet.newEmpty() + patchB.added["A"] = { Id = "A", ClassName = "Folder", Name = "A" } + + expect(PatchSet.isEqual(patchA, patchB)).to.equal(true) + end) + + it("should return false for different patches", function() + local patchA = PatchSet.newEmpty() + patchA.added["A"] = { Id = "A", ClassName = "Folder", Name = "A" } + + local patchB = PatchSet.newEmpty() + patchB.added["B"] = { Id = "B", ClassName = "Folder", Name = "B" } + + expect(PatchSet.isEqual(patchA, patchB)).to.equal(false) + end) + end) +end diff --git a/plugin/src/ServeSession.lua b/plugin/src/ServeSession.lua index adee9e4d..5d140ba0 100644 --- a/plugin/src/ServeSession.lua +++ b/plugin/src/ServeSession.lua @@ -104,6 +104,10 @@ function ServeSession.new(options) __instanceMap = instanceMap, __changeBatcher = changeBatcher, __statusChangedCallback = nil, + __userConfirmCallback = nil, + __patchUpdateCallback = nil, + __serverInfo = nil, + __confirmingPatch = nil, __connections = connections, __precommitCallbacks = {}, __postcommitCallbacks = {}, @@ -138,6 +142,10 @@ function ServeSession:setConfirmCallback(callback) self.__userConfirmCallback = callback end +function ServeSession:setPatchUpdateCallback(callback) + self.__patchUpdateCallback = callback +end + function ServeSession:setUpdateLoadingTextCallback(callback) self.__updateLoadingText = callback end @@ -188,6 +196,62 @@ function ServeSession:hookPostcommit(callback) end end +function ServeSession:__onWebSocketMessage(messagesPacket) + if self.__status == Status.Disconnected then + return + end + + Log.debug("Received {} messages from Rojo server", #messagesPacket.messages) + + -- Combine all messages into a single patch + local combinedPatch = PatchSet.newEmpty() + for _, message in messagesPacket.messages do + PatchSet.assign(combinedPatch, message) + end + + -- If we're already waiting for confirmation, merge into the patch being confirmed + if self.__confirmingPatch ~= nil then + Log.trace("Already confirming, merging into current patch") + PatchSet.merge(self.__confirmingPatch, combinedPatch, self.__instanceMap) + -- Notify the UI to update the displayed patch + if self.__patchUpdateCallback ~= nil then + self.__patchUpdateCallback(self.__instanceMap, self.__confirmingPatch) + end + self.__apiContext:setMessageCursor(messagesPacket.messageCursor) + return + end + + -- Set confirming patch before spawning to prevent race condition + -- where another message arrives before the spawned thread starts + self.__confirmingPatch = combinedPatch + + -- Spawn a new thread to handle potentially yielding confirmation + task.spawn(function() + Log.trace("Processing WebSocket patch, callback exists: {}", self.__userConfirmCallback ~= nil) + Log.trace("ServerInfo exists: {}", self.__serverInfo ~= nil) + + local userDecision = "Accept" + if self.__userConfirmCallback ~= nil then + userDecision = self.__userConfirmCallback(self.__instanceMap, combinedPatch, self.__serverInfo) + end + self.__confirmingPatch = nil + + Log.trace("WebSocket patch decision: {}", userDecision) + + if userDecision == "Abort" then + self:__stopInternal("Aborted Rojo sync operation") + return + elseif userDecision == "Accept" then + -- combinedPatch may have been updated with additional changes + -- that arrived while we were waiting for confirmation + self:__applyPatch(combinedPatch) + end + -- Note: "Reject" is not handled for ongoing patches, only for initial sync + end) + + self.__apiContext:setMessageCursor(messagesPacket.messageCursor) +end + function ServeSession:start() self:__setStatus(Status.Connecting) self:setLoadingText("Connecting to server...") @@ -196,25 +260,26 @@ function ServeSession:start() :connect() :andThen(function(serverInfo) self:setLoadingText("Loading initial data from server...") - return self:__initialSync(serverInfo):andThen(function() + self.__serverInfo = serverInfo + return self:__computeInitialPatch(serverInfo):andThen(function(catchUpPatch) self:setLoadingText("Starting sync loop...") - self:__setStatus(Status.Connected, serverInfo.projectName) - self:__applyGameAndPlaceId(serverInfo) - return self.__apiContext:connectWebSocket({ + -- Connect WebSocket BEFORE showing confirmation so changes + -- during confirmation can be merged into the patch + -- Note: connectWebSocket returns a Promise that only resolves when + -- the connection closes, so we don't chain .andThen() on it + self.__apiContext:connectWebSocket({ ["messages"] = function(messagesPacket) - if self.__status == Status.Disconnected then - return - end - - Log.debug("Received {} messages from Rojo server", #messagesPacket.messages) - - for _, message in messagesPacket.messages do - self:__applyPatch(message) - end - self.__apiContext:setMessageCursor(messagesPacket.messageCursor) + self:__onWebSocketMessage(messagesPacket) end, - }) + }):catch(function(err) + if self.__status ~= Status.Disconnected then + self:__stopInternal(err) + end + end) + + -- Now show confirmation and wait for user decision + return self:__confirmAndApplyInitialPatch(catchUpPatch, serverInfo) end) end) :catch(function(err) @@ -465,7 +530,7 @@ function ServeSession:__applyPatch(patch) end end -function ServeSession:__initialSync(serverInfo) +function ServeSession:__computeInitialPatch(serverInfo) return self.__apiContext:read({ serverInfo.rootInstanceId }):andThen(function(readResponseBody) -- Tell the API Context that we're up-to-date with the version of -- the tree defined in this response. @@ -503,52 +568,62 @@ function ServeSession:__initialSync(serverInfo) Log.trace("Computed hydration patch: {:#?}", debugPatch(catchUpPatch)) - local userDecision = "Accept" - if self.__userConfirmCallback ~= nil then - userDecision = self.__userConfirmCallback(self.__instanceMap, catchUpPatch, serverInfo) - end - - if userDecision == "Abort" then - return Promise.reject("Aborted Rojo sync operation") - elseif userDecision == "Reject" then - if not self.__twoWaySync then - return Promise.reject("Cannot reject sync operation without two-way sync enabled") - end - -- The user wants their studio DOM to write back to their Rojo DOM - -- so we will reverse the patch and send it back - - local inversePatch = PatchSet.newEmpty() - - -- Send back the current properties - for _, change in catchUpPatch.updated do - local instance = self.__instanceMap.fromIds[change.id] - if not instance then - continue - end - - local update = encodePatchUpdate(instance, change.id, change.changedProperties) - table.insert(inversePatch.updated, update) - end - -- Add the removed instances back to Rojo - -- selene:allow(empty_if, unused_variable, empty_loop) - for _, instance in catchUpPatch.removed do - -- TODO: Generate ID for our instance and add it to inversePatch.added - end - -- Remove the additions we've rejected - for id, _change in catchUpPatch.added do - table.insert(inversePatch.removed, id) - end - - return self.__apiContext:write(inversePatch) - elseif userDecision == "Accept" then - self:__applyPatch(catchUpPatch) - return Promise.resolve() - else - return Promise.reject("Invalid user decision: " .. userDecision) - end + return catchUpPatch end) end +function ServeSession:__confirmAndApplyInitialPatch(catchUpPatch, serverInfo) + local userDecision = "Accept" + if self.__userConfirmCallback ~= nil then + -- Set confirming patch so WebSocket messages that arrive during + -- confirmation can be merged into this patch + self.__confirmingPatch = catchUpPatch + userDecision = self.__userConfirmCallback(self.__instanceMap, catchUpPatch, serverInfo) + self.__confirmingPatch = nil + end + + if userDecision == "Abort" then + return Promise.reject("Aborted Rojo sync operation") + elseif userDecision == "Reject" then + if not self.__twoWaySync then + return Promise.reject("Cannot reject sync operation without two-way sync enabled") + end + -- The user wants their studio DOM to write back to their Rojo DOM + -- so we will reverse the patch and send it back + + local inversePatch = PatchSet.newEmpty() + + -- Send back the current properties + for _, change in catchUpPatch.updated do + local instance = self.__instanceMap.fromIds[change.id] + if not instance then + continue + end + + local update = encodePatchUpdate(instance, change.id, change.changedProperties) + table.insert(inversePatch.updated, update) + end + -- Add the removed instances back to Rojo + -- selene:allow(empty_if, unused_variable, empty_loop) + for _, instance in catchUpPatch.removed do + -- TODO: Generate ID for our instance and add it to inversePatch.added + end + -- Remove the additions we've rejected + for id, _change in catchUpPatch.added do + table.insert(inversePatch.removed, id) + end + + return self.__apiContext:write(inversePatch) + elseif userDecision == "Accept" then + self:__setStatus(Status.Connected, serverInfo.projectName) + self:__applyGameAndPlaceId(serverInfo) + self:__applyPatch(catchUpPatch) + return Promise.resolve() + else + return Promise.reject("Invalid user decision: " .. userDecision) + end +end + function ServeSession:__stopInternal(err) self:__setStatus(Status.Disconnected, err) self.__apiContext:disconnect() diff --git a/plugin/src/Settings.lua b/plugin/src/Settings.lua index 64f32241..2c7008d9 100644 --- a/plugin/src/Settings.lua +++ b/plugin/src/Settings.lua @@ -20,7 +20,7 @@ local defaultSettings = { checkForUpdates = true, checkForPrereleases = false, autoConnectPlaytestServer = false, - confirmationBehavior = "Initial" :: "Never" | "Initial" | "Large Changes" | "Unlisted PlaceId", + confirmationBehavior = "Initial" :: "Never" | "Initial" | "Always" | "Large Changes" | "Unlisted PlaceId", largeChangesConfirmationThreshold = 5, playSounds = true, typecheckingEnabled = false,