From 55ac231cecb7f350b8810cb8bcc57fc36a396a47 Mon Sep 17 00:00:00 2001 From: boatbomber Date: Fri, 30 Jun 2023 11:21:36 -0700 Subject: [PATCH] Skip confirming patches that only contain a datamodel name change (#688) Closes #672. Skips the user confirmation if the patch contains only a datamodel name change. I decided to build generic PatchSet utility functions in case we need to use similar logic in the future, in addition to maintaining clear division of duties. The app code shouldn't be too dependent upon patch internal structure when we can avoid it. --------- Co-authored-by: Kenneth Loeffler --- CHANGELOG.md | 2 + plugin/src/App/init.lua | 19 +++++++ plugin/src/PatchSet.lua | 109 +++++++++++++++++++++++++++++++++++++++- 3 files changed, 129 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 68eb44b0..07a20cf9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,10 +4,12 @@ * Significantly improved performance of `rojo sourcemap`. ([#668]) * Fixed the diff visualizer of connected sessions. ([#674]) * Fixed disconnected session activity. ([#675]) +* Skip confirming patches that contain only a datamodel name change. ([#688]) [#668]: https://github.com/rojo-rbx/rojo/pull/668 [#674]: https://github.com/rojo-rbx/rojo/pull/674 [#675]: https://github.com/rojo-rbx/rojo/pull/675 +[#688]: https://github.com/rojo-rbx/rojo/pull/688 ## [7.3.0] - April 22, 2023 * Added `$attributes` to project format. ([#574]) diff --git a/plugin/src/App/init.lua b/plugin/src/App/init.lua index 8c705449..9d58b661 100644 --- a/plugin/src/App/init.lua +++ b/plugin/src/App/init.lua @@ -295,9 +295,28 @@ function App:startSession() serveSession:setConfirmCallback(function(instanceMap, patch, serverInfo) if PatchSet.isEmpty(patch) then + Log.trace("Accepting patch without confirmation because it is empty") return "Accept" end + -- The datamodel name gets overwritten by Studio, making confirmation of it intrusive + -- and unnecessary. This special case allows it to be accepted without confirmation. + if + PatchSet.hasAdditions(patch) == false + and PatchSet.hasRemoves(patch) == false + and PatchSet.containsOnlyInstance(patch, instanceMap, game) + then + local datamodelUpdates = PatchSet.getUpdateForInstance(patch, instanceMap, game) + if + datamodelUpdates ~= nil + and next(datamodelUpdates.changedProperties) == nil + and datamodelUpdates.changedClassName == nil + then + Log.trace("Accepting patch without confirmation because it only contains a datamodel name change") + return "Accept" + end + end + self:setState({ appStatus = AppStatus.Confirming, confirmData = { diff --git a/plugin/src/PatchSet.lua b/plugin/src/PatchSet.lua index 3451de6a..c9f621dc 100644 --- a/plugin/src/PatchSet.lua +++ b/plugin/src/PatchSet.lua @@ -91,6 +91,113 @@ function PatchSet.hasUpdates(patchSet) return next(patchSet.updated) ~= nil end +--[[ + Tells whether the given PatchSet contains changes to the given instance id +]] +function PatchSet.containsId(patchSet, instanceMap, id) + if patchSet.added[id] ~= nil then + return true + end + + for _, idOrInstance in patchSet.removed do + local removedId = if Types.RbxId(idOrInstance) then idOrInstance else instanceMap.fromInstances[idOrInstance] + if removedId == id then + return true + end + end + + for _, update in patchSet.updated do + if update.id == id then + return true + end + end + + return false +end + +--[[ + Tells whether the given PatchSet contains changes to the given instance. + If the given InstanceMap does not contain the instance, this function always returns false. +]] +function PatchSet.containsInstance(patchSet, instanceMap, instance) + local id = instanceMap.fromInstances[instance] + if id == nil then + return false + end + + return PatchSet.containsId(patchSet, instanceMap, id) +end + +--[[ + Tells whether the given PatchSet contains changes to nothing but the given instance id +]] +function PatchSet.containsOnlyId(patchSet, instanceMap, id) + if not PatchSet.containsId(patchSet, instanceMap, id) then + -- Patch doesn't contain the id at all + return false + end + + for addedId in patchSet.added do + if addedId ~= id then + return false + end + end + + for _, idOrInstance in patchSet.removed do + local removedId = if Types.RbxId(idOrInstance) then idOrInstance else instanceMap.fromInstances[idOrInstance] + if removedId ~= id then + return false + end + end + + for _, update in patchSet.updated do + if update.id ~= id then + return false + end + end + + return true +end + +--[[ + Tells whether the given PatchSet contains changes to nothing but the given instance. + If the given InstanceMap does not contain the instance, this function always returns false. +]] +function PatchSet.containsOnlyInstance(patchSet, instanceMap, instance) + local id = instanceMap.fromInstances[instance] + if id == nil then + return false + end + + return PatchSet.containsOnlyId(patchSet, instanceMap, id) +end + +--[[ + Returns the update to the given instance id, or nil if there aren't any +]] +function PatchSet.getUpdateForId(patchSet, id) + for _, update in patchSet.updated do + if update.id == id then + return update + end + end + + return nil +end + +--[[ + Returns the update to the given instance, or nil if there aren't any. + If the given InstanceMap does not contain the instance, this function always returns nil. +]] +function PatchSet.getUpdateForInstance(patchSet, instanceMap, instance) + local id = instanceMap.fromInstances[instance] + if id == nil then + return nil + end + + return PatchSet.getUpdateForId(patchSet, id) +end + --[[ Tells whether the given PatchSets are equal. ]] @@ -150,7 +257,7 @@ function PatchSet.humanSummary(instanceMap, patchSet) for _, idOrInstance in ipairs(patchSet.removed) do local instance, id - if type(idOrInstance) == "string" then + if Types.RbxId(idOrInstance) then id = idOrInstance instance = instanceMap.fromIds[id] else