diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c090788..cb267117 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ Making a new release? Simply add the new header with the version and date undern * Fixed a bug where the notification timeout thread would fail to cancel on unmount ([#1211]) * Added a "Forget" option to the sync reminder notification to avoid being reminded for that place in the future ([#1215]) * Improves relative path calculation for sourcemap generation to avoid issues with Windows UNC paths. ([#1217]) +* Fixed the sync fallback scrambling sibling order; replacements are now re-parented ancestors-first and in their original child order. ([#1265]) [#1176]: https://github.com/rojo-rbx/rojo/pull/1176 [#1179]: https://github.com/rojo-rbx/rojo/pull/1179 @@ -48,6 +49,7 @@ Making a new release? Simply add the new header with the version and date undern [#1211]: https://github.com/rojo-rbx/rojo/pull/1211 [#1215]: https://github.com/rojo-rbx/rojo/pull/1215 [#1217]: https://github.com/rojo-rbx/rojo/pull/1217 +[#1265]: https://github.com/rojo-rbx/rojo/pull/1265 ## [7.7.0-rc.1] (November 27th, 2025) diff --git a/plugin/run-tests.server.lua b/plugin/run-tests.server.lua index 0ab3cc05..aa5daa9f 100644 --- a/plugin/run-tests.server.lua +++ b/plugin/run-tests.server.lua @@ -1,8 +1,8 @@ local ReplicatedStorage = game:GetService("ReplicatedStorage") -local TestEZ = require(ReplicatedStorage.Packages:WaitForChild("TestEZ", 10)) +local TestEZ = require(ReplicatedStorage:WaitForChild("Packages", 10):WaitForChild("TestEZ", 10)) -local Rojo = ReplicatedStorage.Rojo +local Rojo = ReplicatedStorage:WaitForChild("Rojo", 10) local Settings = require(Rojo.Plugin.Settings) Settings:set("logLevel", "Trace") diff --git a/plugin/src/ServeSession.lua b/plugin/src/ServeSession.lua index adee9e4d..7e9fc742 100644 --- a/plugin/src/ServeSession.lua +++ b/plugin/src/ServeSession.lua @@ -18,6 +18,7 @@ local PatchSet = require(script.Parent.PatchSet) local Reconciler = require(script.Parent.Reconciler) local strict = require(script.Parent.strict) local Settings = require(script.Parent.Settings) +local orderSwaps = require(script.Parent.orderSwaps) local Status = strict("Session.Status", { NotStarted = "NotStarted", @@ -320,6 +321,14 @@ function ServeSession:__replaceInstances(idList) return false end + -- Roblox appends to GetChildren() on every reparent, so the order in which + -- we re-parent replacements determines their final sibling order. + -- We process ancestors before descendants (so each replacement's + -- parent already exists when we re-parent it) and siblings in their original + -- GetChildren() order. Because the loop below moves the old instance's + -- children into the replacement *before* re-parenting the replacement, this + -- rebuilds GetChildren() exactly as it was before the swap. + local swaps = {} for id, replacement in replacements do local oldInstance = self.__instanceMap.fromIds[id] if not oldInstance then @@ -328,6 +337,16 @@ function ServeSession:__replaceInstances(idList) continue end + table.insert(swaps, { + id = id, + replacement = replacement, + oldInstance = oldInstance, + }) + end + + for _, swap in orderSwaps(swaps) do + local id, replacement, oldInstance = swap.id, swap.replacement, swap.oldInstance + self.__instanceMap:insert(id, replacement) Log.trace("Swapping Instance {} out via api/models/ endpoint", id) local oldParent = oldInstance.Parent diff --git a/plugin/src/orderSwaps.lua b/plugin/src/orderSwaps.lua new file mode 100644 index 00000000..aa4a438e --- /dev/null +++ b/plugin/src/orderSwaps.lua @@ -0,0 +1,44 @@ +--[[ + Determines the order in which `ServeSession:__replaceInstances` should swap + instances so that sibling order is preserved. + + Roblox appends to `GetChildren()` on every reparent, so the order in which we + re-parent replacements determines their final sibling order. To rebuild + `GetChildren()` exactly as it was before the swap we must: + + * process ancestors before descendants, so each replacement's parent already + exists when we re-parent the replacement, and + * process siblings in their original `GetChildren()` order. + + `swaps` is an array of `{ id, replacement, oldInstance }` entries. This sorts + the array in place (annotating each entry with `depth`/`siblingIndex`) and + returns it. +]] +local function orderSwaps(swaps) + for _, swap in swaps do + local depth = 0 + local ancestor = swap.oldInstance.Parent + while ancestor ~= nil do + depth += 1 + ancestor = ancestor.Parent + end + swap.depth = depth + + local siblingIndex = 0 + if swap.oldInstance.Parent ~= nil then + siblingIndex = table.find(swap.oldInstance.Parent:GetChildren(), swap.oldInstance) or 0 + end + swap.siblingIndex = siblingIndex + end + + table.sort(swaps, function(a, b) + if a.depth ~= b.depth then + return a.depth < b.depth + end + return a.siblingIndex < b.siblingIndex + end) + + return swaps +end + +return orderSwaps diff --git a/plugin/src/orderSwaps.spec.lua b/plugin/src/orderSwaps.spec.lua new file mode 100644 index 00000000..04c730fd --- /dev/null +++ b/plugin/src/orderSwaps.spec.lua @@ -0,0 +1,57 @@ +return function() + local orderSwaps = require(script.Parent.orderSwaps) + + it("orders same-named siblings by their original GetChildren order", function() + local parent = Instance.new("Model") + local a1 = Instance.new("Part") + a1.Name = "a" + a1.Parent = parent + local a2 = Instance.new("Part") + a2.Name = "a" + a2.Parent = parent + local a3 = Instance.new("Part") + a3.Name = "a" + a3.Parent = parent + + -- Input deliberately out of sibling order. + -- orderSwaps must restore the GetChildren() order. + local ordered = orderSwaps({ + { id = "3", oldInstance = a3 }, + { id = "1", oldInstance = a1 }, + { id = "2", oldInstance = a2 }, + }) + + expect(ordered[1].oldInstance).to.equal(a1) + expect(ordered[2].oldInstance).to.equal(a2) + expect(ordered[3].oldInstance).to.equal(a3) + end) + + it("orders ancestors before descendants", function() + local root = Instance.new("Model") + local child = Instance.new("Folder") + child.Parent = root + local grandchild = Instance.new("Part") + grandchild.Parent = child + + local ordered = orderSwaps({ + { id = "grandchild", oldInstance = grandchild }, + { id = "child", oldInstance = child }, + { id = "root", oldInstance = root }, + }) + + expect(ordered[1].oldInstance).to.equal(root) + expect(ordered[2].oldInstance).to.equal(child) + expect(ordered[3].oldInstance).to.equal(grandchild) + end) + + it("returns a single swap unchanged", function() + local part = Instance.new("Part") + + local ordered = orderSwaps({ + { id = "1", oldInstance = part }, + }) + + expect(#ordered).to.equal(1) + expect(ordered[1].oldInstance).to.equal(part) + end) +end