mirror of
https://github.com/rojo-rbx/rojo.git
synced 2026-10-22 21:41:37 +00:00
Complete audit plan updates and enhance deduplication processes
- Updated the audit plan for the ambiguous paths follow-up, marking all critical fixes as completed, including dedup cleanup, added paths, and ref path finalization. - Introduced a new `atlas-dedup.mdc` file detailing the deduplication system, including matching algorithms, suffix cleanup, and integration for duplicate-named instances. - Enhanced the `syncback` process to ensure proper handling of ref paths and deduplication, improving metadata consistency and overall system reliability. - Implemented attribute-scoped path fixes in the `FsSnapshot` to ensure only relevant lines are modified during ref path updates. These changes improve the clarity and effectiveness of the audit process while enhancing the deduplication and synchronization mechanisms within the project.
This commit is contained in:
Joe
parent
ff48e05914
commit
ed1f61c093
11 changed files with 1226 additions and 123 deletions
|
|
@ -4,19 +4,19 @@ overview: Production audit of the ambiguous-paths branch against origin/master.
|
||||||
todos:
|
todos:
|
||||||
- id: fix-metadata-drift
|
- id: fix-metadata-drift
|
||||||
content: "Fix 1: Update dedup cleanup rename flow to keep tree metadata/path indexes in sync"
|
content: "Fix 1: Update dedup cleanup rename flow to keep tree metadata/path indexes in sync"
|
||||||
status: in_progress
|
status: completed
|
||||||
- id: fix-batch-removal-dedup
|
- id: fix-batch-removal-dedup
|
||||||
content: "Fix 2: Exclude co-removed siblings from dedup cleanup sibling enumeration"
|
content: "Fix 2: Exclude co-removed siblings from dedup cleanup sibling enumeration"
|
||||||
status: pending
|
status: completed
|
||||||
- id: fix-added-paths
|
- id: fix-added-paths
|
||||||
content: "Fix 3: Extend added_paths in api.rs to include middleware extension and dedup suffix"
|
content: "Fix 3: Extend added_paths in api.rs to include middleware extension and dedup suffix"
|
||||||
status: pending
|
status: completed
|
||||||
- id: fix-ref-path-final
|
- id: fix-ref-path-final
|
||||||
content: "Fix 4: Complete Phase 2 of previous audit Fix 3 -- pass final_paths to collect_referents instead of global text rewrite"
|
content: "Fix 4: Complete Phase 2 of previous audit Fix 3 -- pass final_paths to collect_referents instead of global text rewrite"
|
||||||
status: pending
|
status: completed
|
||||||
- id: fix-regression-tests
|
- id: fix-regression-tests
|
||||||
content: "Fix 5: Add regression tests for Fixes 1-4"
|
content: "Fix 5: Add regression tests for Fixes 1-4"
|
||||||
status: pending
|
status: completed
|
||||||
isProject: false
|
isProject: false
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -177,7 +177,7 @@ for id, data in patch.added do instanceMap:insert(id, Reconciler.reify(data)) en
|
||||||
for _, update in patch.updated do Reconciler.hydrate(instanceMap:fromId(update.id), update) end
|
for _, update in patch.updated do Reconciler.hydrate(instanceMap:fromId(update.id), update) end
|
||||||
```
|
```
|
||||||
|
|
||||||
## Two-Way Sync (Experimental)
|
## Two-Way Sync
|
||||||
|
|
||||||
**Setting:** `twoWaySync` (default: true) | Syncs Studio changes → filesystem in real-time
|
**Setting:** `twoWaySync` (default: true) | Syncs Studio changes → filesystem in real-time
|
||||||
|
|
||||||
|
|
@ -229,13 +229,21 @@ for _, update in patch.updated do Reconciler.hydrate(instanceMap:fromId(update.i
|
||||||
│ │ Looks up InstigatingSource::Path → fs::remove_file() │
|
│ │ Looks up InstigatingSource::Path → fs::remove_file() │
|
||||||
│ ▼ │
|
│ ▼ │
|
||||||
│ For each added: syncback_added_instance() │
|
│ For each added: syncback_added_instance() │
|
||||||
|
│ │ Groups by parent, pre-seeds sibling_slugs, dedup on collision │
|
||||||
│ │ Determines file format, writes to filesystem │
|
│ │ Determines file format, writes to filesystem │
|
||||||
│ ▼ │
|
│ ▼ │
|
||||||
|
│ Build added_paths map (for same-batch Ref resolution) │
|
||||||
|
│ │ Maps Ref → filesystem path via added_instance_fs_segment() │
|
||||||
|
│ ▼ │
|
||||||
|
│ For each updated: syncback_updated_properties() │
|
||||||
|
│ │ Extracts Ref props → Rojo_Ref_* attrs (path from tree/added_paths) │
|
||||||
|
│ │ merge_or_build_meta() writes props/attrs (supports attr removal) │
|
||||||
|
│ ▼ │
|
||||||
│ Sends PatchSet to tree_mutation_sender channel │
|
│ Sends PatchSet to tree_mutation_sender channel │
|
||||||
│ ▼ │
|
│ ▼ │
|
||||||
│ ChangeProcessor::handle_tree_event() [src/change_processor.rs] │
|
│ ChangeProcessor::handle_tree_event() [src/change_processor.rs] │
|
||||||
│ │ For removed: fs::remove_file(path) + remove meta │
|
│ │ For removed: dedup cleanup (gap-tolerant, base-name promotion) │
|
||||||
│ │ For updated: Source writes, renames, ClassName changes, properties │
|
│ │ For updated: Source writes, renames, ClassName changes, ref paths │
|
||||||
│ ▼ │
|
│ ▼ │
|
||||||
│ apply_patch_set() → Updates RojoTree │
|
│ apply_patch_set() → Updates RojoTree │
|
||||||
│ ▼ │
|
│ ▼ │
|
||||||
|
|
@ -293,17 +301,19 @@ end
|
||||||
|
|
||||||
| File | Purpose |
|
| File | Purpose |
|
||||||
|------|---------|
|
|------|---------|
|
||||||
| `init.lua` | Main batcher: accumulator, RenderStepped loop, pause/resume |
|
| `init.lua` | Main batcher: accumulator, RenderStepped loop, pause/resume, deferred refs |
|
||||||
| `createPatchSet.lua` | Converts pending changes → PatchSet |
|
| `createPatchSet.lua` | Converts pending changes → PatchSet (passes instanceMap for Ref resolution) |
|
||||||
| `encodePatchUpdate.lua` | Encodes property updates for existing instances |
|
| `encodePatchUpdate.lua` | Encodes property updates including Ref properties via InstanceMap |
|
||||||
| `encodeInstance.lua` | Full instance encoding (for additions/syncback) |
|
| `encodeInstance.lua` | Full instance encoding (for additions/syncback) |
|
||||||
| `encodeProperty.lua` | Single property value encoding via RbxDom |
|
| `encodeProperty.lua` | Single property value encoding via RbxDom |
|
||||||
|
| `propertyFilter.lua` | Property type filtering (only `UniqueId` blocked; `Ref` is allowed) |
|
||||||
|
|
||||||
**Core State:**
|
**Core State:**
|
||||||
```lua
|
```lua
|
||||||
ChangeBatcher = {
|
ChangeBatcher = {
|
||||||
__accumulator = 0, -- Time since last flush
|
__accumulator = 0, -- Time since last flush
|
||||||
__pendingPropertyChanges = {}, -- Instance → { PropertyName → true }
|
__pendingPropertyChanges = {}, -- Instance → { PropertyName → true }
|
||||||
|
__pendingUnresolvedRefs = {}, -- targetInstance → [{sourceInstance, propertyName}]
|
||||||
__syncSourceOnly = false, -- Only sync Source property
|
__syncSourceOnly = false, -- Only sync Source property
|
||||||
__paused = false, -- Pause during confirmation
|
__paused = false, -- Pause during confirmation
|
||||||
__renderSteppedConnection = conn, -- Frame loop connection
|
__renderSteppedConnection = conn, -- Frame loop connection
|
||||||
|
|
@ -338,10 +348,20 @@ end
|
||||||
- Reduces server load and WebSocket traffic
|
- Reduces server load and WebSocket traffic
|
||||||
- Still feels responsive to users
|
- Still feels responsive to users
|
||||||
|
|
||||||
|
**Deferred Ref Mechanism:**
|
||||||
|
|
||||||
|
When a Ref property targets an instance not yet in the InstanceMap (e.g., newly pulled instance whose server ID hasn't arrived), the property is deferred:
|
||||||
|
|
||||||
|
```lua
|
||||||
|
-- In encodePatchUpdate: onUnresolvedRef callback defers the property
|
||||||
|
-- In ChangeBatcher: __pendingUnresolvedRefs[targetInstance] = {sourceInstance, propertyName}
|
||||||
|
-- When target appears (onInstanceInserted), re-queues the property change for next batch
|
||||||
|
```
|
||||||
|
|
||||||
### createPatchSet.lua: Patch Generation
|
### createPatchSet.lua: Patch Generation
|
||||||
|
|
||||||
```lua
|
```lua
|
||||||
function createPatchSet(instanceMap, propertyChanges, syncSourceOnly)
|
function createPatchSet(instanceMap, propertyChanges, syncSourceOnly, onUnresolvedRef)
|
||||||
local patch = PatchSet.newEmpty()
|
local patch = PatchSet.newEmpty()
|
||||||
|
|
||||||
for instance, properties in propertyChanges do
|
for instance, properties in propertyChanges do
|
||||||
|
|
@ -353,20 +373,18 @@ function createPatchSet(instanceMap, propertyChanges, syncSourceOnly)
|
||||||
|
|
||||||
if properties.Parent then
|
if properties.Parent then
|
||||||
if instance.Parent == nil then
|
if instance.Parent == nil then
|
||||||
-- Instance was deleted in Studio
|
|
||||||
table.insert(patch.removed, instanceId)
|
table.insert(patch.removed, instanceId)
|
||||||
else
|
else
|
||||||
Log.warn("Cannot sync non-nil Parent changes yet") -- Reparenting not supported
|
Log.warn("Cannot sync non-nil Parent changes yet")
|
||||||
end
|
end
|
||||||
else
|
else
|
||||||
if syncSourceOnly then
|
if syncSourceOnly then
|
||||||
-- Server in source-only mode: only sync Source property
|
|
||||||
if properties.Source then
|
if properties.Source then
|
||||||
local update = encodePatchUpdate(instance, instanceId, { Source = true })
|
local update = encodePatchUpdate(instance, instanceId, { Source = true }, instanceMap, onUnresolvedRef)
|
||||||
if update then table.insert(patch.updated, update) end
|
if update then table.insert(patch.updated, update) end
|
||||||
end
|
end
|
||||||
else
|
else
|
||||||
local update = encodePatchUpdate(instance, instanceId, properties)
|
local update = encodePatchUpdate(instance, instanceId, properties, instanceMap, onUnresolvedRef)
|
||||||
if update then table.insert(patch.updated, update) end
|
if update then table.insert(patch.updated, update) end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
@ -378,8 +396,10 @@ end
|
||||||
|
|
||||||
### encodePatchUpdate.lua: Update Encoding
|
### encodePatchUpdate.lua: Update Encoding
|
||||||
|
|
||||||
|
Encodes property updates for existing instances. Now handles Ref properties via InstanceMap lookup.
|
||||||
|
|
||||||
```lua
|
```lua
|
||||||
function encodePatchUpdate(instance, instanceId, properties)
|
function encodePatchUpdate(instance, instanceId, properties, instanceMap, onUnresolvedRef)
|
||||||
local update = {
|
local update = {
|
||||||
id = instanceId,
|
id = instanceId,
|
||||||
changedProperties = {},
|
changedProperties = {},
|
||||||
|
|
@ -392,15 +412,30 @@ function encodePatchUpdate(instance, instanceId, properties)
|
||||||
local descriptor = RbxDom.findCanonicalPropertyDescriptor(instance.ClassName, propertyName)
|
local descriptor = RbxDom.findCanonicalPropertyDescriptor(instance.ClassName, propertyName)
|
||||||
if not descriptor then continue end
|
if not descriptor then continue end
|
||||||
|
|
||||||
local success, encoded = encodeProperty(instance, propertyName, descriptor)
|
if descriptor.dataType == "Ref" then
|
||||||
if success then
|
-- Ref property: resolve target via InstanceMap
|
||||||
update.changedProperties[propertyName] = encoded
|
local target = instance[propertyName]
|
||||||
|
if target == nil then
|
||||||
|
update.changedProperties[propertyName] = { Ref = NULL_REF }
|
||||||
|
else
|
||||||
|
local targetId = instanceMap.fromInstances[target]
|
||||||
|
if targetId then
|
||||||
|
update.changedProperties[propertyName] = { Ref = targetId }
|
||||||
|
elseif onUnresolvedRef then
|
||||||
|
onUnresolvedRef(target, instance, propertyName)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
else
|
||||||
|
local success, encoded = encodeProperty(instance, propertyName, descriptor)
|
||||||
|
if success then
|
||||||
|
update.changedProperties[propertyName] = encoded
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
if next(update.changedProperties) == nil and update.changedName == nil then
|
if next(update.changedProperties) == nil and update.changedName == nil then
|
||||||
return nil -- No actual changes
|
return nil
|
||||||
end
|
end
|
||||||
|
|
||||||
return update
|
return update
|
||||||
|
|
@ -409,47 +444,25 @@ end
|
||||||
|
|
||||||
### encodeInstance.lua: Full Instance Encoding (Syncback/Additions)
|
### encodeInstance.lua: Full Instance Encoding (Syncback/Additions)
|
||||||
|
|
||||||
Used when pulling new instances from Studio to filesystem. **Critical: handles duplicate detection.**
|
Used when pulling new instances from Studio to filesystem. Duplicate-named instances are encoded and sent to the server; the server handles filesystem collisions via the dedup suffix system (`~N`).
|
||||||
|
|
||||||
```lua
|
```lua
|
||||||
-- Duplicate handling (matches server's ref_properties.rs::is_path_unique)
|
|
||||||
local function isPathUnique(instance)
|
|
||||||
local current = instance
|
|
||||||
while current do
|
|
||||||
if hasDuplicateSiblings(current) then
|
|
||||||
return false -- Any duplicate in ancestor chain = ambiguous path
|
|
||||||
end
|
|
||||||
current = current.Parent
|
|
||||||
if current == game then break end
|
|
||||||
end
|
|
||||||
return true
|
|
||||||
end
|
|
||||||
|
|
||||||
function encodeInstance(instance, parentId, _skipPathCheck)
|
function encodeInstance(instance, parentId, _skipPathCheck)
|
||||||
-- Skip instances with ambiguous paths (duplicate-named siblings)
|
|
||||||
if not _skipPathCheck and not isPathUnique(instance) then
|
|
||||||
Log.warn("Skipped '{}' - path contains duplicate-named siblings", instance:GetFullName())
|
|
||||||
return nil
|
|
||||||
end
|
|
||||||
|
|
||||||
local properties = {}
|
local properties = {}
|
||||||
|
|
||||||
-- Encode Attributes and Tags (work on all instance types)
|
|
||||||
encodeAttributes(instance, properties)
|
encodeAttributes(instance, properties)
|
||||||
encodeTags(instance, properties)
|
encodeTags(instance, properties)
|
||||||
|
|
||||||
-- For scripts, Source is required
|
|
||||||
if SCRIPT_CLASS_NAMES[instance.ClassName] then
|
if SCRIPT_CLASS_NAMES[instance.ClassName] then
|
||||||
local success, encoded = encodeProperty(instance, "Source", descriptor)
|
local success, encoded = encodeProperty(instance, "Source", descriptor)
|
||||||
if not success then return nil end
|
if not success then return nil end
|
||||||
properties.Source = encoded
|
properties.Source = encoded
|
||||||
end
|
end
|
||||||
|
|
||||||
-- Encode all serializable properties
|
|
||||||
local classDescriptor = RbxDom.findClassDescriptor(instance.ClassName)
|
local classDescriptor = RbxDom.findClassDescriptor(instance.ClassName)
|
||||||
for propertyName, meta in classDescriptor.properties do
|
for propertyName, meta in classDescriptor.properties do
|
||||||
if SKIP_PROPERTIES[propertyName] then continue end -- Parent, Name, Archivable
|
if SKIP_PROPERTIES[propertyName] then continue end
|
||||||
if SKIP_PROPERTIES_BY_CLASS[instance.ClassName]?[propertyName] then continue end -- Source, ScriptGuid, etc.
|
if SKIP_PROPERTIES_BY_CLASS[instance.ClassName]?[propertyName] then continue end
|
||||||
|
|
||||||
local isReadable = meta.scriptability == "ReadWrite" or meta.scriptability == "Read"
|
local isReadable = meta.scriptability == "ReadWrite" or meta.scriptability == "Read"
|
||||||
local doesSerialize = meta.serialization ~= "DoesNotSerialize"
|
local doesSerialize = meta.serialization ~= "DoesNotSerialize"
|
||||||
|
|
@ -462,12 +475,11 @@ function encodeInstance(instance, parentId, _skipPathCheck)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
-- Recursively encode children (skipping duplicates at each level)
|
-- All children encoded (including those with duplicate names;
|
||||||
|
-- the server handles duplicates via dedup suffix system)
|
||||||
local children = {}
|
local children = {}
|
||||||
local childDuplicates = findDuplicateNames(instance:GetChildren())
|
|
||||||
for _, child in instance:GetChildren() do
|
for _, child in instance:GetChildren() do
|
||||||
if childDuplicates[child.Name] then continue end
|
local encoded = encodeInstance(child, nil, true)
|
||||||
local encoded = encodeInstance(child, nil, true) -- Skip path check, already validated
|
|
||||||
if encoded then table.insert(children, encoded) end
|
if encoded then table.insert(children, encoded) end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
@ -521,19 +533,31 @@ true, false, 42, "string"
|
||||||
|
|
||||||
### Server-Side: ChangeProcessor (`src/change_processor.rs`)
|
### Server-Side: ChangeProcessor (`src/change_processor.rs`)
|
||||||
|
|
||||||
Receives patches from plugin via `tree_mutation_receiver` channel. Handles removals, Source writes, renames (with slugify/dedup/meta lifecycle), ClassName changes (script type transitions via file renames), and property writes to meta/model files.
|
Receives patches from plugin via `tree_mutation_receiver` channel. Handles removals (with dedup cleanup), Source writes, renames (with slugify/dedup/meta lifecycle), ClassName changes (script type transitions via file renames), property writes to meta/model files, and ref path updates.
|
||||||
|
|
||||||
**Capabilities:**
|
**Capabilities:**
|
||||||
- **Removals:** Deletes file + associated meta file via `InstigatingSource::Path`
|
- **Removals:** Deletes file + associated meta file via `InstigatingSource::Path`
|
||||||
|
- **Dedup cleanup after removals:** When a deletion leaves a dedup group needing suffix cleanup, performs renames (gap-tolerant, base-name promotion, group-to-1). Uses `removed_set` (HashSet) to exclude co-removed siblings from sibling enumeration. Calls `compute_cleanup_action()` from `dedup_suffix.rs`.
|
||||||
- **Source writes:** Updates script file contents
|
- **Source writes:** Updates script file contents
|
||||||
- **Renames:** Slugifies new name, deduplicates against siblings, renames file on disk, creates/updates/removes meta `name` field as needed. Handles both standalone files and init-file directories.
|
- **Renames:** Slugifies new name, deduplicates against siblings, renames file on disk, creates/updates/removes meta `name` field as needed. Handles both standalone files and init-file directories.
|
||||||
- **ClassName changes:** Script-to-script transitions (e.g. ModuleScript → Script) via file extension renames
|
- **ClassName changes:** Script-to-script transitions (e.g. ModuleScript → Script) via file extension renames
|
||||||
- **Property writes:** Attributes and other properties written to meta/model files via `syncback_updated_properties` in `api.rs`
|
- **Property writes:** Attributes and other properties written to meta/model files via `syncback_updated_properties` in `api.rs`
|
||||||
|
- **Ref path updates:** `RefPathIndex` tracks which meta/model files contain `Rojo_Ref_*` paths. `update_ref_paths_after_rename()` efficiently updates all affected files when an instance is renamed or dedup-cleaned.
|
||||||
|
- **VFS suppression for dedup renames:** `suppress_path_any(&from)` + `suppress_path(&to)` prevents the VFS watcher from re-triggering on files renamed during dedup cleanup.
|
||||||
|
- **InstigatingSource updates:** After dedup cleanup renames, the renamed sibling's `InstigatingSource::Path` and metadata path indexes are updated in-memory.
|
||||||
|
|
||||||
**InstigatingSource:** Every instance in RojoTree tracks where it came from:
|
**InstigatingSource:** Every instance in RojoTree tracks where it came from:
|
||||||
- `InstigatingSource::Path(PathBuf)` - Created from a file, can be written back
|
- `InstigatingSource::Path(PathBuf)` - Created from a file, can be written back
|
||||||
- `InstigatingSource::ProjectNode { path, name, node, parent_class }` - Defined in project file, **cannot be modified via two-way sync**
|
- `InstigatingSource::ProjectNode { path, name, node, parent_class }` - Defined in project file, **cannot be modified via two-way sync**
|
||||||
|
|
||||||
|
**`handle_tree_event()` flow:**
|
||||||
|
1. Process removals (log but don't delete -- API already deleted files)
|
||||||
|
2. Dedup cleanup (runs before `apply_patch_set` to use parent/sibling relationships)
|
||||||
|
3. Process updates: renames, ClassName changes, Source writes, ref path updates
|
||||||
|
4. `apply_patch_set()` to update RojoTree
|
||||||
|
5. Apply metadata updates (InstigatingSource paths for both regular and dedup renames)
|
||||||
|
6. Send applied patch to message queue for plugin broadcast
|
||||||
|
|
||||||
### Pause Mechanism: Critical for Consistency
|
### Pause Mechanism: Critical for Consistency
|
||||||
|
|
||||||
**Why pause is essential:**
|
**Why pause is essential:**
|
||||||
|
|
@ -592,8 +616,21 @@ end
|
||||||
| **Rapid changes interleave** | 200ms batch window may split logically related changes | Partial updates can leave inconsistent state |
|
| **Rapid changes interleave** | 200ms batch window may split logically related changes | Partial updates can leave inconsistent state |
|
||||||
| **Reparenting not supported** | `Parent` property changes to non-nil values are ignored | Moving instances in Studio doesn't sync |
|
| **Reparenting not supported** | `Parent` property changes to non-nil values are ignored | Moving instances in Studio doesn't sync |
|
||||||
| **Project node instances immutable** | InstigatingSource::ProjectNode cannot be modified | Instances from project file can't be changed |
|
| **Project node instances immutable** | InstigatingSource::ProjectNode cannot be modified | Instances from project file can't be changed |
|
||||||
| **Duplicate-named siblings skipped** | Path ambiguity makes reliable sync impossible | Instances with same-name siblings ignored |
|
| **Greedy matching not globally optimal** | Ambiguous groups (same Name+ClassName) use greedy assignment | Rare edge cases may produce suboptimal pairing and rename churn |
|
||||||
| **Ref properties complex** | Instance references may become invalid when instances move | ObjectValue.Value may not sync correctly |
|
| **Ref paths with ancestor-level ambiguity** | `Rojo_Ref_*` path-based refs written to disk may resolve to wrong sibling if duplicates exist at any ancestor level | Ref may point to wrong instance after rebuild; in-memory tree is correct during session |
|
||||||
|
|
||||||
|
### Reconciler Matching Algorithm (`plugin/src/Reconciler/`)
|
||||||
|
|
||||||
|
The plugin uses a recursive change-count scoring algorithm (implemented in `matching.lua`) to pair incoming virtual instances with existing Studio instances during hydration.
|
||||||
|
|
||||||
|
**Entry point:** `Matching.matchChildren(virtualChildren, studioChildren, parentVirtual, parentStudio)`
|
||||||
|
|
||||||
|
**Algorithm:**
|
||||||
|
1. **Fast path:** Group by (Name, ClassName). 1:1 groups get instant-matched with zero scoring.
|
||||||
|
2. **Ambiguous groups:** For groups with multiple candidates, compute pairwise `_computeChangeCount()` (recursive, scores entire subtree). Greedy assignment sorted by cost ascending, child-order tiebreaker.
|
||||||
|
3. **Leftovers:** Unmatched virtual = new instances, unmatched Studio = deleted.
|
||||||
|
|
||||||
|
**Value equality:** Shared `trueEquals.lua` module used by both `matching.lua` and `diff.lua` (fuzzy equality for floats, Color3, CFrame, Vector3; NaN handling; nil/null-ref equivalence).
|
||||||
|
|
||||||
## Protocol Enforcement
|
## Protocol Enforcement
|
||||||
|
|
||||||
|
|
@ -637,4 +674,5 @@ Browser: `http://localhost:34872` → Network tab → WebSocket messages
|
||||||
| `src/web/api.rs` - Endpoints | `plugin/src/ServeSession.lua` - Connection |
|
| `src/web/api.rs` - Endpoints | `plugin/src/ServeSession.lua` - Connection |
|
||||||
| `src/web/interface.rs` - Packets | `plugin/src/ApiContext.lua` - HTTP/WS client |
|
| `src/web/interface.rs` - Packets | `plugin/src/ApiContext.lua` - HTTP/WS client |
|
||||||
| `src/serve_session.rs` - Session | `plugin/src/ChangeBatcher/` - Two-way sync |
|
| `src/serve_session.rs` - Session | `plugin/src/ChangeBatcher/` - Two-way sync |
|
||||||
| | `plugin/src/Reconciler/` - Patch application |
|
| `src/change_processor.rs` - FS writer | `plugin/src/Reconciler/` - Patch application |
|
||||||
|
| `src/rojo_ref.rs` - Ref paths | `plugin/src/Reconciler/matching.lua` - Matching |
|
||||||
|
|
|
||||||
556
.cursor/rules/atlas-dedup.mdc
Normal file
556
.cursor/rules/atlas-dedup.mdc
Normal file
|
|
@ -0,0 +1,556 @@
|
||||||
|
---
|
||||||
|
description: "Dedup system: matching algorithm, filesystem-collision dedup, suffix cleanup, ref path system, slugification, and cross-system integration for duplicate-named instances."
|
||||||
|
globs:
|
||||||
|
- "src/syncback/file_names.rs"
|
||||||
|
- "src/syncback/dedup_suffix.rs"
|
||||||
|
- "src/syncback/matching.rs"
|
||||||
|
- "src/syncback/ref_properties.rs"
|
||||||
|
- "src/snapshot/matching.rs"
|
||||||
|
- "src/snapshot/tree.rs"
|
||||||
|
- "src/snapshot/patch_compute.rs"
|
||||||
|
- "src/rojo_ref.rs"
|
||||||
|
- "src/variant_eq.rs"
|
||||||
|
- "src/change_processor.rs"
|
||||||
|
- "src/web/api.rs"
|
||||||
|
- "plugin/src/Reconciler/matching.lua"
|
||||||
|
- "plugin/src/Reconciler/trueEquals.lua"
|
||||||
|
- "plugin/src/Reconciler/hydrate.lua"
|
||||||
|
- "plugin/src/Reconciler/diff.lua"
|
||||||
|
- "plugin/src/XXH32.luau"
|
||||||
|
- "tests/tests/two_way_sync.rs"
|
||||||
|
- "tests/tests/syncback_roundtrip.rs"
|
||||||
|
---
|
||||||
|
|
||||||
|
# Atlas Dedup System
|
||||||
|
|
||||||
|
> Atlas is a fork of Rojo. Many internal identifiers, API endpoints, and struct names still reference the original "Rojo" naming.
|
||||||
|
|
||||||
|
## Quality Standard
|
||||||
|
|
||||||
|
This is a production-grade synchronization system. The filesystem must represent a 1:1 copy of the Studio place. The core invariant:
|
||||||
|
|
||||||
|
**Round-trip identity:** Syncback (or two-way sync) writes a directory tree. Building an rbxl from that directory tree and forward-syncing it back must produce a bit-identical instance tree -- same names, same classes, same properties, same hierarchy, same ref targets. Any deviation is a bug.
|
||||||
|
|
||||||
|
**CLI syncback parity:** Plugin-based two-way sync must produce exactly the same filesystem output as `atlas syncback`. Byte-for-byte identical files, identical directory structures, identical naming. Any divergence is a bug until proven otherwise.
|
||||||
|
|
||||||
|
## Architecture Overview
|
||||||
|
|
||||||
|
The dedup system replaced the old "skip duplicates" blacklist with three interlocking subsystems:
|
||||||
|
|
||||||
|
```
|
||||||
|
Matching Algorithm → Dedup System → Ref Path System
|
||||||
|
(pairs instances) (assigns FS names) (paths use final FS names)
|
||||||
|
```
|
||||||
|
|
||||||
|
**Execution order per sync operation:** Matching runs first (pairs instances), dedup runs second (assigns filesystem names), ref paths are computed last (using final filesystem names).
|
||||||
|
|
||||||
|
### Three Implementations of Matching
|
||||||
|
|
||||||
|
The matching algorithm exists in 3 parallel implementations that must produce identical pairings:
|
||||||
|
|
||||||
|
1. **Rust syncback** -- `src/syncback/matching.rs` (operates on `WeakDom` instances)
|
||||||
|
2. **Rust forward sync** -- `src/snapshot/matching.rs` (operates on `InstanceSnapshot` vs `RojoTree`)
|
||||||
|
3. **Lua plugin** -- `plugin/src/Reconciler/matching.lua` (operates on Studio instances vs virtual instances)
|
||||||
|
|
||||||
|
All three use the same recursive change-count scoring + greedy assignment algorithm. Changes to the algorithm must be applied to all three.
|
||||||
|
|
||||||
|
### Key Files Map
|
||||||
|
|
||||||
|
| File | Subsystem | Purpose |
|
||||||
|
|------|-----------|---------|
|
||||||
|
| `src/syncback/file_names.rs` | Dedup | `slugify_name`, `deduplicate_name_with_ext`, `name_for_inst`, `parse_dedup_suffix` |
|
||||||
|
| `src/syncback/dedup_suffix.rs` | Dedup | `compute_cleanup_action`, `DedupCleanupAction`, suffix cleanup rules |
|
||||||
|
| `src/syncback/matching.rs` | Matching | Syncback matching: `match_children` (WeakDom) |
|
||||||
|
| `src/snapshot/matching.rs` | Matching | Forward sync matching: `match_forward` (Snapshot vs Tree) |
|
||||||
|
| `plugin/src/Reconciler/matching.lua` | Matching | Lua matching: `matchChildren` (Studio vs Virtual) |
|
||||||
|
| `src/rojo_ref.rs` | Ref Paths | `ref_target_path_from_tree`, `RefPathIndex`, `Rojo_Ref_*` constants |
|
||||||
|
| `src/snapshot/tree.rs` | Ref Paths | `get_instance_by_path`, `filesystem_name_for` |
|
||||||
|
| `src/syncback/ref_properties.rs` | Ref Paths | `collect_referents`, `tentative_fs_path`, `RefLinks` |
|
||||||
|
| `src/variant_eq.rs` | Matching | `variant_eq` property comparison (Rust) |
|
||||||
|
| `plugin/src/Reconciler/trueEquals.lua` | Matching | `trueEquals` property comparison (Lua) |
|
||||||
|
| `src/change_processor.rs` | Integration | Dedup cleanup on deletion, VFS suppression, ref path updates |
|
||||||
|
| `src/web/api.rs` | Integration | `added_paths`, `syncback_updated_properties`, ref-to-attribute conversion |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Subsystem 1: Slugification and Dedup (`file_names.rs`)
|
||||||
|
|
||||||
|
### slugify_name
|
||||||
|
|
||||||
|
Pure, stateless. Replaces forbidden filesystem characters with `_`.
|
||||||
|
|
||||||
|
**Constants:**
|
||||||
|
```rust
|
||||||
|
const FORBIDDEN_CHARS: [char; 9] = ['<', '>', ':', '"', '/', '|', '?', '*', '\\'];
|
||||||
|
const SLUGIFY_CHARS: [char; 10] = ['<', '>', ':', '"', '/', '|', '?', '*', '\\', '~'];
|
||||||
|
const DANGEROUS_SUFFIXES: [&str; 8] = [".server", ".client", ".plugin", ".local", ".legacy", ".meta", ".model", ".project"];
|
||||||
|
const INVALID_WINDOWS_NAMES: [&str; 22] = ["CON", "PRN", "AUX", "NUL", "COM1".."COM9", "LPT1".."LPT9"];
|
||||||
|
```
|
||||||
|
|
||||||
|
**Behavior:**
|
||||||
|
- Replaces each `SLUGIFY_CHARS` char with `_` (including `~` to prevent natural names from looking like dedup suffixes)
|
||||||
|
- Strips leading/trailing spaces and dots
|
||||||
|
- Neutralizes Windows reserved names (`CON` → `CON_`)
|
||||||
|
- Neutralizes dangerous suffixes (`.server` etc.) that would be misinterpreted as middleware extensions
|
||||||
|
|
||||||
|
**Key invariant:** `slugify_name` is idempotent. Applying it twice produces the same result.
|
||||||
|
|
||||||
|
### name_needs_slugify
|
||||||
|
|
||||||
|
Returns `true` if the name contains any `SLUGIFY_CHARS`. Used to decide whether metadata `name` field is needed.
|
||||||
|
|
||||||
|
### deduplicate_name_with_ext
|
||||||
|
|
||||||
|
Stateful collision resolver. Takes a base slug and extension, checks against `taken_names` set (case-insensitive).
|
||||||
|
|
||||||
|
```rust
|
||||||
|
pub fn deduplicate_name_with_ext(
|
||||||
|
base: &str,
|
||||||
|
extension: Option<&str>,
|
||||||
|
taken_names: &HashSet<String>,
|
||||||
|
) -> (String, String)
|
||||||
|
// Returns: (deduped_slug, full_fs_name)
|
||||||
|
// "Foo" + ".server.luau" with collision → ("Foo~1", "Foo~1.server.luau")
|
||||||
|
```
|
||||||
|
|
||||||
|
**Key invariant:** Suffix goes on the stem BEFORE the compound extension: `Foo~1.server.luau`, NOT `Foo.server~1.luau`.
|
||||||
|
|
||||||
|
### Dedup Key Semantics
|
||||||
|
|
||||||
|
The dedup key is the **full filesystem name component** (slug + extension), lowercased. This means:
|
||||||
|
|
||||||
|
- `Folder "Foo"` (key: `"foo"`) and `Script "Foo"` (key: `"foo.server.luau"`) do NOT collide
|
||||||
|
- `Script "Foo" (server)` and `Script "Foo" (server)` both key `"foo.server.luau"` → collision, dedup applied
|
||||||
|
- `Folder "Foo"` and init-style `Script "Foo"` both key `"foo"` (both directories) → collision, dedup applied
|
||||||
|
|
||||||
|
### name_for_inst
|
||||||
|
|
||||||
|
The primary entry point for generating filesystem names during syncback:
|
||||||
|
|
||||||
|
```rust
|
||||||
|
pub fn name_for_inst<'a>(
|
||||||
|
middleware: Middleware,
|
||||||
|
new_inst: &'a Instance,
|
||||||
|
old_inst: Option<InstanceWithMeta<'a>>,
|
||||||
|
taken_names: &HashSet<String>,
|
||||||
|
) -> anyhow::Result<(Cow<'a, str>, bool, String)>
|
||||||
|
// Returns: (filename, needs_meta_name, dedup_key)
|
||||||
|
```
|
||||||
|
|
||||||
|
- In incremental mode (`old_inst` is `Some`): preserves existing filesystem path
|
||||||
|
- In clean mode: slugifies name, adds middleware extension, deduplicates against `taken_names`
|
||||||
|
- Returns `needs_meta_name = true` when slug differs from real name (including dedup suffix)
|
||||||
|
|
||||||
|
### parse_dedup_suffix
|
||||||
|
|
||||||
|
Parses `~N` suffix from a stem: `"Foo~3"` → `Some(("Foo", 3))`, `"Foo"` → `None`.
|
||||||
|
|
||||||
|
### Metadata `name` Field
|
||||||
|
|
||||||
|
When the filesystem name differs from the instance name (due to slugification or dedup), the real name is stored in `.meta.json5` or `.model.json5` `name` field. This is the authoritative instance name during forward sync.
|
||||||
|
|
||||||
|
```
|
||||||
|
Instance "Hey/Bro" → Hey_Bro.luau + meta: {"name": "Hey/Bro"}
|
||||||
|
Instance "Folder" → Folder/ (no meta needed, name == slug)
|
||||||
|
Instance "Folder" → Folder~1/ + meta: {"name": "Folder"}
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Subsystem 2: Dedup Suffix Cleanup (`dedup_suffix.rs`)
|
||||||
|
|
||||||
|
When instances are deleted, the remaining dedup group may need suffix adjustments.
|
||||||
|
|
||||||
|
### DedupCleanupAction
|
||||||
|
|
||||||
|
```rust
|
||||||
|
pub enum DedupCleanupAction {
|
||||||
|
None,
|
||||||
|
RemoveSuffix { from: PathBuf, to: PathBuf },
|
||||||
|
PromoteLowest { from: PathBuf, to: PathBuf },
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
### Three Cleanup Rules
|
||||||
|
|
||||||
|
1. **Gap-tolerant deletion:** Deleting `~1` from `{Foo, Foo~1, Foo~2}` leaves `{Foo, Foo~2}`. No renumbering. Gaps are tolerated.
|
||||||
|
|
||||||
|
2. **Base-name promotion:** Deleting `Foo` (base) from `{Foo, Foo~1, Foo~2}` promotes `Foo~1` to `Foo`. `Foo~2` stays. The lowest-numbered suffix becomes the new base.
|
||||||
|
|
||||||
|
3. **Group-to-1 cleanup:** Reducing the group to 1 member removes the suffix entirely. Deleting from `{Foo, Foo~1}` leaves one instance → renamed to clean `Foo`.
|
||||||
|
|
||||||
|
### compute_cleanup_action
|
||||||
|
|
||||||
|
```rust
|
||||||
|
pub fn compute_cleanup_action(
|
||||||
|
base_stem: &str,
|
||||||
|
extension: Option<&str>,
|
||||||
|
remaining_stems: &[String],
|
||||||
|
deleted_was_base: bool,
|
||||||
|
parent_dir: &Path,
|
||||||
|
) -> DedupCleanupAction
|
||||||
|
```
|
||||||
|
|
||||||
|
**Critical integration detail:** The `remaining_stems` input must exclude co-removed siblings (instances being deleted in the same batch). The `change_processor.rs` builds a `removed_set: HashSet<Ref>` and skips those refs during sibling enumeration.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Subsystem 3: Matching Algorithm
|
||||||
|
|
||||||
|
### Entry Points
|
||||||
|
|
||||||
|
**Rust syncback:**
|
||||||
|
```rust
|
||||||
|
pub fn match_children(
|
||||||
|
new_children: &[Ref], old_children: &[Ref],
|
||||||
|
new_dom: &WeakDom, old_dom: &WeakDom,
|
||||||
|
new_hashes: Option<&HashMap<Ref, Hash>>,
|
||||||
|
old_hashes: Option<&HashMap<Ref, Hash>>,
|
||||||
|
) -> MatchResult
|
||||||
|
```
|
||||||
|
|
||||||
|
**Rust forward sync:**
|
||||||
|
```rust
|
||||||
|
pub fn match_forward(
|
||||||
|
snapshot_children: Vec<InstanceSnapshot>,
|
||||||
|
tree_children: &[Ref],
|
||||||
|
tree: &RojoTree,
|
||||||
|
) -> ForwardMatchResult
|
||||||
|
```
|
||||||
|
|
||||||
|
**Lua plugin:**
|
||||||
|
```lua
|
||||||
|
function matchChildren(virtualInstances, virtualRootId, instanceMap, instance) -> MatchResult
|
||||||
|
```
|
||||||
|
|
||||||
|
### Algorithm (All Three Implementations)
|
||||||
|
|
||||||
|
**Fast-path 1: Ref Pin.** Scan parent's Ref properties (PrimaryPart, Value, etc.). If parent A's PrimaryPart points to child X on side A, and parent B's PrimaryPart points to child Y on side B, pin (X, Y) as matched. Remove both.
|
||||||
|
|
||||||
|
**Fast-path 2: (Name, ClassName) Grouping.** Group remaining children by `(Name, ClassName)`. 1:1 groups get instant-matched with zero scoring.
|
||||||
|
|
||||||
|
**Ambiguous Groups: Recursive Scoring + Greedy Assignment.** For groups with multiple candidates on at least one side:
|
||||||
|
|
||||||
|
```
|
||||||
|
for each (A, B) in cross-product:
|
||||||
|
cost = computeChangeCount(A, B, bestSoFar)
|
||||||
|
if cost < bestSoFar: bestSoFar = cost
|
||||||
|
|
||||||
|
stable sort by cost ascending (ties broken by child order)
|
||||||
|
greedy assign: pick lowest-cost pair, remove both, repeat
|
||||||
|
```
|
||||||
|
|
||||||
|
### computeChangeCount (Recursive)
|
||||||
|
|
||||||
|
`computeChangeCount` and `matchChildren` are **mutually recursive**:
|
||||||
|
|
||||||
|
```
|
||||||
|
computeChangeCount(A, B, bestSoFar):
|
||||||
|
cost = countOwnDiffs(A, B) // properties + tags + attributes
|
||||||
|
if cost >= bestSoFar: return cost // early exit
|
||||||
|
|
||||||
|
childResult = matchChildren(A.children, B.children) // RECURSE
|
||||||
|
for matched pair: cost += computeChangeCount(childA, childB, bestSoFar - cost)
|
||||||
|
cost += count(unmatched) * UNMATCHED_PENALTY
|
||||||
|
return cost
|
||||||
|
```
|
||||||
|
|
||||||
|
### Constants
|
||||||
|
|
||||||
|
```rust
|
||||||
|
const UNMATCHED_PENALTY: u32 = 10_000; // Unmatched child = categorically expensive
|
||||||
|
const MAX_SCORING_DEPTH: u32 = 3; // Depth limit prevents O(n^k) explosion
|
||||||
|
```
|
||||||
|
|
||||||
|
### countOwnDiffs
|
||||||
|
|
||||||
|
Counts property-level changes needed to turn A into B:
|
||||||
|
- Each differing property = +1 (using `variant_eq` / `trueEquals`)
|
||||||
|
- Each tag in symmetric difference = +1
|
||||||
|
- Each differing attribute = +1
|
||||||
|
- Name and ClassName NOT counted (grouping guarantees match)
|
||||||
|
|
||||||
|
### Hash Fast-Path (Syncback Only)
|
||||||
|
|
||||||
|
Syncback matching has precomputed blake3 subtree hashes. If `new_hash == old_hash`, return cost 0 immediately (skips property comparison and recursion).
|
||||||
|
|
||||||
|
### Sort Stability
|
||||||
|
|
||||||
|
- **Rust:** `slice::sort_by` is natively stable. Ties preserve child order.
|
||||||
|
- **Lua:** `table.sort` is NOT stable. Comparator breaks ties by insertion index: `if a.cost == b.cost then return a.idx < b.idx end`.
|
||||||
|
|
||||||
|
### Property Comparison
|
||||||
|
|
||||||
|
**Rust (`variant_eq`):** Type check first; float comparison uses `approx_eq!` with epsilon. Special handling for CFrame, Color3, Vector3, ColorSequence, NumberSequence, Attributes (sorted), Tags (sorted).
|
||||||
|
|
||||||
|
**Lua (`trueEquals`):** Recursive deep equality. Treats `nil` and null ref (`{Ref = "000...0"}`) as equal. Floats use epsilon 0.0001 (absolute + relative). CFrame/Vector3 component-wise. Color3 via RGB ints. EnumItem via `.Value`.
|
||||||
|
|
||||||
|
### Known Limitation: Greedy vs Optimal
|
||||||
|
|
||||||
|
The greedy algorithm is NOT globally optimal. Edge cases exist where a locally-best pick forces a globally-worse assignment. The Hungarian algorithm (O(N^3)) would give optimal results, but ambiguous groups are typically small (2-10 instances). Accepted for current release.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Subsystem 4: Ref Path System
|
||||||
|
|
||||||
|
### Path Format
|
||||||
|
|
||||||
|
Ref paths use **full filesystem names** including extensions and dedup suffixes:
|
||||||
|
|
||||||
|
```
|
||||||
|
Folder "Foo" → segment "Foo"
|
||||||
|
ModuleScript "Foo" → segment "Foo.luau"
|
||||||
|
Script "Foo" (server) → segment "Foo.server.luau"
|
||||||
|
Dedup'd ModuleScript → segment "Foo~1.luau"
|
||||||
|
Init-style Script → segment "Foo" (directory)
|
||||||
|
```
|
||||||
|
|
||||||
|
Paths are slash-separated from DataModel's children downward: `"Workspace/TestModel/Part1.model.json5"`.
|
||||||
|
|
||||||
|
### Key Functions
|
||||||
|
|
||||||
|
**Building paths:**
|
||||||
|
```rust
|
||||||
|
// From RojoTree (two-way sync, change_processor):
|
||||||
|
pub fn ref_target_path_from_tree(tree: &RojoTree, target_ref: Ref) -> String
|
||||||
|
// Uses filesystem_name_for() for each ancestor
|
||||||
|
|
||||||
|
// From WeakDom (CLI syncback):
|
||||||
|
pub fn ref_target_path(dom: &WeakDom, target_ref: Ref) -> String
|
||||||
|
// Uses dom.full_path_of()
|
||||||
|
|
||||||
|
// Tentative path (before dedup suffixes assigned):
|
||||||
|
fn tentative_fs_path(dom: &WeakDom, target_ref: Ref) -> String
|
||||||
|
// Uses slugified name + extension based on class
|
||||||
|
```
|
||||||
|
|
||||||
|
**Resolving paths:**
|
||||||
|
```rust
|
||||||
|
// In tree.rs:
|
||||||
|
pub fn get_instance_by_path(&self, path: &str) -> Option<Ref>
|
||||||
|
// Splits by "/", matches segments case-insensitive against filesystem_name_for()
|
||||||
|
// Fallback: matches against instance name (case-insensitive)
|
||||||
|
|
||||||
|
pub fn filesystem_name_for(&self, id: Ref) -> String
|
||||||
|
// Returns filename from instigating_source, falls back to instance name
|
||||||
|
```
|
||||||
|
|
||||||
|
**Computing filesystem names for refs:**
|
||||||
|
```rust
|
||||||
|
// In ref_properties.rs:
|
||||||
|
fn tentative_fs_name(inst: &Instance) -> String
|
||||||
|
// Slugified name + extension based on class and children count
|
||||||
|
// Used before dedup suffixes are assigned
|
||||||
|
|
||||||
|
pub fn collect_referents(
|
||||||
|
dom: &WeakDom,
|
||||||
|
pre_prune_paths: &HashMap<Ref, String>,
|
||||||
|
final_paths: Option<&HashMap<Ref, String>>,
|
||||||
|
) -> RefLinks
|
||||||
|
// Uses final_paths (dedup-aware) when available, otherwise tentative_fs_path
|
||||||
|
```
|
||||||
|
|
||||||
|
### Attribute Format
|
||||||
|
|
||||||
|
Ref properties are stored as `Rojo_Ref_*` attributes in meta/model files:
|
||||||
|
|
||||||
|
```json5
|
||||||
|
{
|
||||||
|
"attributes": {
|
||||||
|
"Rojo_Ref_PrimaryPart": "Workspace/TestModel/Part1.model.json5"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Constants in `rojo_ref.rs`:
|
||||||
|
```rust
|
||||||
|
pub const REF_PATH_ATTRIBUTE_PREFIX: &str = "Rojo_Ref_";
|
||||||
|
pub const REF_POINTER_ATTRIBUTE_PREFIX: &str = "Rojo_Target_";
|
||||||
|
pub const REF_ID_ATTRIBUTE_NAME: &str = "Rojo_Id";
|
||||||
|
```
|
||||||
|
|
||||||
|
### RefPathIndex
|
||||||
|
|
||||||
|
Tracks which filesystem files contain which ref paths, enabling efficient updates on rename:
|
||||||
|
|
||||||
|
```rust
|
||||||
|
pub struct RefPathIndex {
|
||||||
|
paths_to_files: HashMap<String, HashSet<PathBuf>>,
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Used by `update_ref_paths_after_rename` in `change_processor.rs` to find and update all files containing a given ref path prefix when a dedup rename occurs.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Integration Point 1: change_processor.rs (Dedup Cleanup)
|
||||||
|
|
||||||
|
### Deletion Flow
|
||||||
|
|
||||||
|
When instances are removed via two-way sync, `handle_tree_event` runs dedup cleanup **before** `apply_patch_set` removes instances from the tree:
|
||||||
|
|
||||||
|
1. Build `removed_set: HashSet<Ref>` from ALL removals in the batch
|
||||||
|
2. For each removed instance:
|
||||||
|
a. Get parent ref and filesystem name
|
||||||
|
b. Parse dedup suffix from stem
|
||||||
|
c. Enumerate remaining siblings (skipping co-removed via `removed_set`)
|
||||||
|
d. Group by extension (prevents cross-middleware false matches)
|
||||||
|
e. Call `compute_cleanup_action`
|
||||||
|
3. Execute cleanup action (rename file, suppress VFS, update ref paths)
|
||||||
|
|
||||||
|
### VFS Suppression
|
||||||
|
|
||||||
|
Dedup renames must suppress VFS events to prevent feedback loops:
|
||||||
|
- `suppress_path_any(&old_path)` -- old path may trigger REMOVE (Linux/Windows) or stale CREATE (macOS)
|
||||||
|
- `suppress_path(&new_path)` -- new path triggers Create/Write
|
||||||
|
- `unsuppress_path(&new_path)` -- rollback if rename fails
|
||||||
|
|
||||||
|
**Critical:** New instance additions must NOT be suppressed. The VFS watcher needs to pick up new files for forward sync.
|
||||||
|
|
||||||
|
### Ref Path Updates After Dedup Rename
|
||||||
|
|
||||||
|
After successful dedup rename (e.g., `Foo~1` → `Foo`):
|
||||||
|
1. Compute old and new ref path segments
|
||||||
|
2. Call `update_ref_paths_after_rename(&old_prefix, &new_prefix)`
|
||||||
|
3. This function uses `RefPathIndex` to find affected files
|
||||||
|
4. Updates `Rojo_Ref_*` attribute values in each affected file
|
||||||
|
5. Updates the index: both path keys and filesystem paths
|
||||||
|
6. Suppresses VFS events for each file write
|
||||||
|
|
||||||
|
### Metadata Consistency
|
||||||
|
|
||||||
|
After dedup rename, the survivor's metadata must be updated:
|
||||||
|
- `InstigatingSource::Path` updated to new filesystem path
|
||||||
|
- Path indexes updated in-memory
|
||||||
|
- Suppression counters kept consistent with updated path
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Integration Point 2: api.rs (Two-Way Sync Write)
|
||||||
|
|
||||||
|
### added_paths (Same-Batch Ref Targets)
|
||||||
|
|
||||||
|
When `/api/write` adds instances and sets Ref properties targeting them in the same batch, `added_paths` pre-computes filesystem paths:
|
||||||
|
|
||||||
|
```rust
|
||||||
|
let added_paths: HashMap<Ref, String> = request.added.iter()
|
||||||
|
.filter_map(|(guid, added)| {
|
||||||
|
let parent_path = ref_target_path_from_tree(&tree, added.parent?);
|
||||||
|
let fs_name = added_instance_fs_segment(added); // slug + extension
|
||||||
|
Some((*guid, format!("{}/{}", parent_path, fs_name)))
|
||||||
|
})
|
||||||
|
.collect();
|
||||||
|
```
|
||||||
|
|
||||||
|
`added_instance_fs_segment` mirrors `tentative_fs_name` logic but operates on `AddedInstance` fields (class_name, children, properties).
|
||||||
|
|
||||||
|
### Ref-to-Attribute Conversion
|
||||||
|
|
||||||
|
`syncback_updated_properties` converts `Variant::Ref` properties to `Rojo_Ref_*` attributes:
|
||||||
|
|
||||||
|
1. Extract Ref properties from `changed_properties` BEFORE `filter_properties_for_meta`
|
||||||
|
2. For each Ref property:
|
||||||
|
- **Nil ref:** Mark `Rojo_Ref_PropertyName` for removal
|
||||||
|
- **Valid target in tree:** Compute path via `ref_target_path_from_tree`, add to `ref_attributes`
|
||||||
|
- **Target in `added_paths`:** Use pre-computed path
|
||||||
|
- **Target not found:** Log warning and skip
|
||||||
|
3. Merge `ref_attributes` into attributes map
|
||||||
|
4. Pass `remove_attributes` to `merge_or_build_meta`
|
||||||
|
|
||||||
|
### merge_or_build_meta Attribute Lifecycle
|
||||||
|
|
||||||
|
```
|
||||||
|
1. Remove stale attributes (remove_attributes list)
|
||||||
|
2. Merge new attributes (including Rojo_Ref_* from ref properties)
|
||||||
|
3. Remove wins over add if same key in both lists? No -- adds happen AFTER removes, so add wins
|
||||||
|
```
|
||||||
|
|
||||||
|
### syncback_added_instance
|
||||||
|
|
||||||
|
When adding instances, dedup is handled via `sibling_slugs: &mut HashSet<String>`:
|
||||||
|
- Tracks filesystem names claimed in the current batch
|
||||||
|
- Prevents duplicate slug assignment when multiple instances added simultaneously
|
||||||
|
- Passed to `syncback_instance_to_path_with_stats` for dedup suffix assignment
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Key Invariants
|
||||||
|
|
||||||
|
1. **Round-trip identity:** syncback → build → compare = bit-identical trees
|
||||||
|
2. **CLI syncback parity:** two-way sync produces byte-identical filesystem output to CLI syncback
|
||||||
|
3. **Determinism:** same input always produces same filesystem output (sorted child ordering, deterministic suffix assignment)
|
||||||
|
4. **InstigatingSource consistency:** dedup'd paths reflected in `InstigatingSource::Path`
|
||||||
|
5. **Compound extensions:** suffix before compound extension (`Foo~1.server.luau`, NOT `Foo.server~1.luau`)
|
||||||
|
6. **Meta pairing:** `Foo~1.server.luau` pairs with `Foo~1.meta.json5` (strip script suffix, keep dedup suffix)
|
||||||
|
7. **Dedup key = full FS name:** collisions checked using complete filesystem name (slug + extension), lowercased
|
||||||
|
8. **Co-removed exclusion:** batch removal skips co-removed siblings when computing cleanup actions
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Known Limitations (Accepted)
|
||||||
|
|
||||||
|
### ProjectNode Duplicate-Named Children
|
||||||
|
|
||||||
|
Duplicate-named children under ProjectNode are currently skipped during syncback with warnings (`src/snapshot_middleware/project.rs`). Project keys are unique by JSON definition, but filesystem children under project nodes could collide. Accepted for current release.
|
||||||
|
|
||||||
|
### Greedy Matching Not Globally Optimal
|
||||||
|
|
||||||
|
The greedy assignment may produce suboptimal pairing in rare ambiguous groups. A globally optimal algorithm (Hungarian, O(N^3)) would be better but is deferred since ambiguous groups are typically small.
|
||||||
|
|
||||||
|
### Matching Cache Not Implemented
|
||||||
|
|
||||||
|
Phase 2e from the original plan: optional cache mapping `parent_ref → (children_list_hash, match_assignments)` to avoid re-running matching when children haven't changed. Optimization only, not needed for correctness.
|
||||||
|
|
||||||
|
### Stale Ref Paths After Rename
|
||||||
|
|
||||||
|
When a Ref target is renamed, the stored `Rojo_Ref_*` path becomes stale. During two-way sync, `update_ref_paths_after_rename` handles this for dedup-triggered renames. However, user-initiated renames via the plugin do NOT automatically update ref paths in meta files. The path remains stale until the next full syncback. This is a known round-trip violation.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Deferred Refactors
|
||||||
|
|
||||||
|
- **Shared filesystem-segment derivation:** Consolidate tentative filesystem name computation into one shared helper used by `rojo_ref.rs`, `tree.rs`, `api.rs`, and `ref_properties.rs`. Currently duplicated across 4+ call sites.
|
||||||
|
- **Optimal assignment algorithm:** Replace greedy with Hungarian for large ambiguous groups.
|
||||||
|
- **Matching cache:** Add per-parent cache with invalidation for performance.
|
||||||
|
- **Duplicated name→index grouping:** All three matching implementations repeat the same HashMap pattern. Could be factored but types differ.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Testing Strategy
|
||||||
|
|
||||||
|
### Test Locations
|
||||||
|
|
||||||
|
- Rust unit tests: `#[cfg(test)]` blocks in source files
|
||||||
|
- Rust integration tests: `tests/tests/two_way_sync.rs`, `tests/tests/syncback_roundtrip.rs`
|
||||||
|
- Test fixtures: `rojo-test/syncback-tests/`, `rojo-test/build-tests/`
|
||||||
|
- Lua spec tests: `.spec.lua` files
|
||||||
|
- Snapshot tests: `cargo insta review`
|
||||||
|
|
||||||
|
### Critical Test Scenarios
|
||||||
|
|
||||||
|
**Dedup:** Same-named Folders, Folders with different children, Folder + Script same name (no dedup), Folder + init-style Script (dedup), natural tilde name, third duplicate added, suffix cleanup (group-to-1), gap tolerance, base-name promotion, idempotency, roundtrip.
|
||||||
|
|
||||||
|
**Ref paths:** Basic ref survives syncback, ref to dedup'd instance, ref survives cleanup rename, ref survives base-name promotion, ref survives group-to-1, deeply nested through dedup layers, cross-file ref update, tilde-named instance ref, two-way sync ref update, forward sync manual edit.
|
||||||
|
|
||||||
|
**Matching:** ClassName discriminator, property similarity scoring, matching stability across runs, preserves existing pairings, fast path verification, recursive depth limit, hash fast-path short circuit.
|
||||||
|
|
||||||
|
**Dedup collision:** Folder + ModuleScript (no collision, different keys), Server + Client script (no collision), ModuleScript + ModuleScript (collision), slug collision ("Foo?" + "Foo*"), case-insensitive collision ("Foo" + "foo").
|
||||||
|
|
||||||
|
**Cross-system:** Full roundtrip with duplicates, two-way sync add/delete, forward sync edit dedup'd file, project file + filesystem duplicates, batch-remove 2 of 3 same-named siblings, same-batch add + ref.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Historical Context
|
||||||
|
|
||||||
|
The dedup system was built across these plans (chronological):
|
||||||
|
|
||||||
|
1. **nuke_path_encoding** -- Replaced `%NAME%` encoding with slugify + dedup + metadata `name` field foundation
|
||||||
|
2. **two-way_sync_refs** -- Added Ref property support to two-way sync with `Rojo_Ref_*` path-based attributes
|
||||||
|
3. **atlas_fork_isolation** -- Renamed binary/plugin for coexistence (preserved `Rojo_Ref_*` attribute names for backward compatibility)
|
||||||
|
4. **ambiguous_path_handling** -- Core feature: 3-pass matching algorithm, filesystem-collision dedup, filesystem-name ref paths, dedup suffix cleanup
|
||||||
|
5. **ref_path_and_dedup_integration** -- Migrated `collect_referents` to filesystem-name paths, integrated cleanup rules into change_processor
|
||||||
|
6. **rewrite_matching_algorithm** -- Replaced multi-pass with recursive change-count scoring + greedy assignment
|
||||||
|
7. **audit_ambiguous_path_handling** -- First audit: fixed forward sync re-matching, case sensitivity, ProjectNode paths, Lua require crash, dead code, ref format mismatch
|
||||||
|
8. **ambiguous_paths_audit_fixes** -- Second audit: recursive matching, `added_paths` format, dedup-aware ref collection, full-FS-name cleanup grouping, shared `trueEquals`
|
||||||
|
9. **audit_ambiguous_paths_followup** -- Third audit: metadata drift after cleanup rename, co-removed sibling exclusion, `added_paths` extension/suffix, attribute-scoped final paths
|
||||||
|
10. **audit_ref_two-way_sync** -- Ref audit: round-trip fidelity, path computation correctness, meta lifecycle, VFS suppression, wire format
|
||||||
|
|
@ -256,6 +256,8 @@ Add metadata to file-based instances. Meta files use **camelCase keys** (no `$`
|
||||||
|
|
||||||
**`name` field (slugify support):** When an instance name contains forbidden filesystem characters (`: / ? * < > | " \`), syncback slugifies the filename (forbidden chars → `_`) and stores the real name in the `name` field. On forward sync, `name` overrides the filename-derived instance name. On collisions (e.g. `A/B` and `A_B` both slugify to `A_B`), `~1`, `~2` suffixes are appended and meta `name` fields distinguish them.
|
**`name` field (slugify support):** When an instance name contains forbidden filesystem characters (`: / ? * < > | " \`), syncback slugifies the filename (forbidden chars → `_`) and stores the real name in the `name` field. On forward sync, `name` overrides the filename-derived instance name. On collisions (e.g. `A/B` and `A_B` both slugify to `A_B`), `~1`, `~2` suffixes are appended and meta `name` fields distinguish them.
|
||||||
|
|
||||||
|
**`Rojo_Ref_*` attributes:** Meta files may also contain `Rojo_Ref_PropertyName` attributes written by syncback/two-way sync for Ref property linking. These are managed automatically and should not be hand-edited.
|
||||||
|
|
||||||
**Meta files CAN'T:** Set `$path`, have children.
|
**Meta files CAN'T:** Set `$path`, have children.
|
||||||
|
|
||||||
## Model.json5 Format
|
## Model.json5 Format
|
||||||
|
|
@ -331,6 +333,27 @@ Same format as properties. Limitations: no Ref, no NumberSequence/ColorSequence,
|
||||||
|
|
||||||
## Ref Property Linking
|
## Ref Property Linking
|
||||||
|
|
||||||
|
Two systems for linking instance references, both supported:
|
||||||
|
|
||||||
|
### Path-Based: `Rojo_Ref_*` (preferred, written by syncback/two-way sync)
|
||||||
|
|
||||||
|
Uses filesystem-name paths (slugified, dedup'd) to reference target instances:
|
||||||
|
|
||||||
|
```json5
|
||||||
|
// In meta file for an instance with PrimaryPart pointing to Handle
|
||||||
|
{
|
||||||
|
"attributes": {
|
||||||
|
"Rojo_Ref_PrimaryPart": "Workspace/MyModel/Handle.model.json5"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Attribute format:** `Rojo_Ref_<PropertyName> = "<filesystem-name path>"`
|
||||||
|
|
||||||
|
Path segments use full filesystem names (slug + extension + dedup suffix). Resolved during forward sync via `get_instance_by_path()` (case-insensitive filesystem name matching with instance name fallback).
|
||||||
|
|
||||||
|
### ID-Based: `Rojo_Target_*` + `$id` (legacy, still read by forward sync)
|
||||||
|
|
||||||
Link instances via `$id` and special attributes:
|
Link instances via `$id` and special attributes:
|
||||||
|
|
||||||
```json5
|
```json5
|
||||||
|
|
@ -347,6 +370,8 @@ Link instances via `$id` and special attributes:
|
||||||
|
|
||||||
**Attribute format:** `Rojo_Target_<PropertyName>`
|
**Attribute format:** `Rojo_Target_<PropertyName>`
|
||||||
|
|
||||||
|
**Priority:** When both `Rojo_Ref_*` and `Rojo_Target_*` exist for the same property, `Rojo_Ref_*` takes precedence.
|
||||||
|
|
||||||
## Property Types
|
## Property Types
|
||||||
|
|
||||||
`String`, `Bool`, `Int32`, `Int64`, `Float32`, `Float64`, `Vector2`, `Vector3`, `Vector2int16`, `Vector3int16`, `UDim`, `UDim2`, `CFrame`, `Color3`, `Color3uint8`, `BrickColor`, `NumberRange`, `Rect`, `NumberSequence`, `ColorSequence`, `Enum`, `Ref`, `Content`, `Font`, `PhysicalProperties`, `Axes`, `Faces`, `Tags`, `Attributes`
|
`String`, `Bool`, `Int32`, `Int64`, `Float32`, `Float64`, `Vector2`, `Vector3`, `Vector2int16`, `Vector3int16`, `UDim`, `UDim2`, `CFrame`, `Color3`, `Color3uint8`, `BrickColor`, `NumberRange`, `Rect`, `NumberSequence`, `ColorSequence`, `Enum`, `Ref`, `Content`, `Font`, `PhysicalProperties`, `Axes`, `Faces`, `Tags`, `Attributes`
|
||||||
|
|
|
||||||
|
|
@ -19,8 +19,9 @@ Pure functions: filesystem → instance tree → minimal patches → apply chang
|
||||||
|
|
||||||
**Key files:**
|
**Key files:**
|
||||||
- `mod.rs` - System overview with detailed explanation
|
- `mod.rs` - System overview with detailed explanation
|
||||||
- `patch_compute.rs` - Generate diffs
|
- `patch_compute.rs` - Generate diffs (uses matching algorithm)
|
||||||
- `patch_apply.rs` - Apply changes
|
- `patch_apply.rs` - Apply changes (resolves `Rojo_Ref_*` and `Rojo_Target_*` refs)
|
||||||
|
- `matching.rs` - Forward sync matching algorithm (recursive change-count scoring)
|
||||||
- `instance_snapshot.rs` - Core snapshot type
|
- `instance_snapshot.rs` - Core snapshot type
|
||||||
|
|
||||||
```rust
|
```rust
|
||||||
|
|
@ -73,9 +74,9 @@ pub struct InstanceContext {
|
||||||
|
|
||||||
**Filename handling (syncback/two-way sync):**
|
**Filename handling (syncback/two-way sync):**
|
||||||
|
|
||||||
Instance names with forbidden filesystem chars (`<>:"/\|?*`) are handled by:
|
Instance names with forbidden filesystem chars (`<>:"/\|?*~`) are handled by:
|
||||||
1. `slugify_name()` -- replaces forbidden chars with `_` (pure, stateless)
|
1. `slugify_name()` -- replaces forbidden chars with `_` (pure, stateless)
|
||||||
2. `deduplicate_name()` -- appends `~1`, `~2`, etc. on collision (stateful)
|
2. `deduplicate_name_with_ext()` -- dedup key = full filesystem name (slug + extension), case-insensitive; appends `~1`, `~2`, etc. on collision (stateful)
|
||||||
3. Metadata `name` field -- stores the real instance name in `.meta.json5`/`.model.json5` when the slug differs
|
3. Metadata `name` field -- stores the real instance name in `.meta.json5`/`.model.json5` when the slug differs
|
||||||
|
|
||||||
### 3. Commands (`src/cli/`)
|
### 3. Commands (`src/cli/`)
|
||||||
|
|
@ -93,6 +94,10 @@ Reverse: Roblox file → filesystem
|
||||||
|
|
||||||
- `snapshot.rs` - Convert instances to snapshots
|
- `snapshot.rs` - Convert instances to snapshots
|
||||||
- `fs_snapshot.rs` - Filesystem representation
|
- `fs_snapshot.rs` - Filesystem representation
|
||||||
|
- `matching.rs` - Syncback matching algorithm (recursive change-count scoring)
|
||||||
|
- `dedup_suffix.rs` - Dedup cleanup rules (gap-tolerant, base-name promotion, group-to-1)
|
||||||
|
- `meta.rs` - Meta file read/write helpers (upsert/remove name, update ref paths)
|
||||||
|
- `ref_properties.rs` - Ref property path linking (`Rojo_Ref_*`, tentative_fs_path)
|
||||||
- `hash/` - Content hashing for change detection
|
- `hash/` - Content hashing for change detection
|
||||||
- `property_filter.rs` - Apply syncback rules
|
- `property_filter.rs` - Apply syncback rules
|
||||||
|
|
||||||
|
|
@ -220,10 +225,10 @@ Review: `cargo insta review`
|
||||||
|
|
||||||
**WebSocket:** `ws://host:port/api/rojo`
|
**WebSocket:** `ws://host:port/api/rojo`
|
||||||
|
|
||||||
**Protocol v5:**
|
**Protocol v6:**
|
||||||
```json
|
```json
|
||||||
// Handshake
|
// Handshake
|
||||||
{"type": "ServerInfo", "protocolVersion": 5, "serverVersion": "7.7.0"}
|
{"type": "ServerInfo", "protocolVersion": 6, "serverVersion": "7.7.0"}
|
||||||
|
|
||||||
// Patch
|
// Patch
|
||||||
{
|
{
|
||||||
|
|
@ -367,13 +372,20 @@ RUST_LOG=rojo=debug rojo build
|
||||||
|
|
||||||
```
|
```
|
||||||
src/
|
src/
|
||||||
cli/ - Commands
|
cli/ - Commands
|
||||||
snapshot/ - Core system
|
snapshot/ - Core system
|
||||||
snapshot_middleware/ - File handlers
|
matching.rs - Forward sync matching algorithm
|
||||||
syncback/ - Reverse sync
|
snapshot_middleware/ - File handlers
|
||||||
web/ - HTTP/WebSocket
|
syncback/ - Reverse sync
|
||||||
lib.rs - Public API
|
matching.rs - Syncback matching algorithm
|
||||||
main.rs - CLI entry
|
dedup_suffix.rs - Dedup cleanup rules
|
||||||
|
meta.rs - Meta file helpers
|
||||||
|
web/ - HTTP/WebSocket
|
||||||
|
change_processor.rs - Two-way sync filesystem writer
|
||||||
|
rojo_ref.rs - Ref path system (Rojo_Ref_*, RefPathIndex)
|
||||||
|
variant_eq.rs - Property value comparison
|
||||||
|
lib.rs - Public API
|
||||||
|
main.rs - CLI entry
|
||||||
|
|
||||||
crates/
|
crates/
|
||||||
memofs/ - Virtual filesystem
|
memofs/ - Virtual filesystem
|
||||||
|
|
@ -48,15 +48,17 @@ Filesystem (Lua files, models, directories)
|
||||||
|
|
||||||
```
|
```
|
||||||
syncback/
|
syncback/
|
||||||
mod.rs - Main loop, orchestration
|
mod.rs - Main loop, orchestration, ref path post-processing
|
||||||
snapshot.rs - Convert instances → snapshots
|
snapshot.rs - Convert instances → snapshots, record_ref_path()
|
||||||
fs_snapshot.rs - Filesystem representation
|
fs_snapshot.rs - Filesystem representation, fix_ref_paths()
|
||||||
file_names.rs - Generate valid filenames
|
file_names.rs - slugify_name(), deduplicate_name_with_ext(), name_for_inst()
|
||||||
meta.rs - Meta file read/write helpers
|
matching.rs - Recursive change-count matching algorithm
|
||||||
|
dedup_suffix.rs - Dedup cleanup rules (gap-tolerant, base-name promotion, group-to-1)
|
||||||
|
meta.rs - Meta file read/write helpers (upsert/remove name, update ref paths)
|
||||||
property_filter.rs - Filter properties per syncback rules
|
property_filter.rs - Filter properties per syncback rules
|
||||||
ref_properties.rs - Handle Ref properties (cross-references)
|
ref_properties.rs - Ref property path linking (Rojo_Ref_*, tentative_fs_path)
|
||||||
hash/ - Content hashing for change detection
|
hash/ - Content hashing for change detection
|
||||||
mod.rs - Hash variants
|
mod.rs - Hash tree computation
|
||||||
variant.rs - Variant-specific hashing
|
variant.rs - Variant-specific hashing
|
||||||
stats.rs - Statistics tracking
|
stats.rs - Statistics tracking
|
||||||
```
|
```
|
||||||
|
|
@ -66,13 +68,16 @@ syncback/
|
||||||
**Flow:**
|
**Flow:**
|
||||||
|
|
||||||
1. **Parse input file** - Read rbxl/rbxm with `rbx_binary`
|
1. **Parse input file** - Read rbxl/rbxm with `rbx_binary`
|
||||||
2. **Prune tree** - Remove instances not in original project
|
2. **Collect pre-prune paths** - `collect_all_paths()` maps all Refs to filesystem-name paths before pruning (for external refs)
|
||||||
3. **Filter hidden services** - Remove internal Roblox services (AdService, Analytics, etc.)
|
3. **Prune tree** - Remove instances not in original project
|
||||||
4. **Collect pre-prune paths** - Track all instances before pruning (for external refs)
|
4. **Filter hidden services** - Remove internal Roblox services (AdService, Analytics, etc.)
|
||||||
5. **Hash comparison** - Compare hashes of old tree vs new tree
|
5. **Collect referents** - `collect_referents()` finds Ref properties and writes `Rojo_Ref_*` attributes using tentative paths
|
||||||
6. **Recursive syncback** - Walk trees, matching instances to files
|
6. **Filter properties** - Apply syncback rules (ignore classes, properties, etc.)
|
||||||
7. **Write filesystem** - Create/update files based on changes
|
7. **Hash comparison** - Compare hashes of old tree vs new tree
|
||||||
8. **Link Ref properties** - Resolve cross-references between instances
|
8. **Recursive syncback walk** - Match instances to files via matching algorithm, assign filenames with dedup
|
||||||
|
9. **Record ref path map** - During the walk, `record_ref_path()` maps each Ref to its final dedup'd filesystem path
|
||||||
|
10. **Post-process ref paths** - `fix_ref_paths()` compares tentative paths vs final paths, substitutes in written meta/model files
|
||||||
|
11. **Write filesystem** - Apply FsSnapshot to disk (sequential or parallel)
|
||||||
|
|
||||||
**Signature:**
|
**Signature:**
|
||||||
```rust
|
```rust
|
||||||
|
|
@ -111,6 +116,41 @@ pub fn syncback_loop(
|
||||||
|
|
||||||
**Implementation detail:** `old_ref` is ignored in `SyncbackSnapshot::with_joined_path` when `incremental=false`.
|
**Implementation detail:** `old_ref` is ignored in `SyncbackSnapshot::with_joined_path` when `incremental=false`.
|
||||||
|
|
||||||
|
## Matching Algorithm (`matching.rs`)
|
||||||
|
|
||||||
|
Pairs new instances (from Roblox file) with old instances (from filesystem) to determine which files to update vs create.
|
||||||
|
|
||||||
|
**Entry point:** `match_children(new_children, old_children, new_dom, old_dom, hashes)`
|
||||||
|
|
||||||
|
**Algorithm:**
|
||||||
|
1. **Fast path:** Group by (Name, ClassName). 1:1 groups get instant-matched with zero scoring.
|
||||||
|
2. **Ambiguous groups:** For groups with multiple candidates on either side, compute pairwise `compute_change_count()` -- a recursive scoring function that counts property diffs + recursive children cost. Greedy assignment sorted by cost ascending, child-order tiebreaker for stability.
|
||||||
|
3. **Hash fast-path:** If `new_hash == old_hash`, return 0 cost immediately (skips property comparison and recursion).
|
||||||
|
|
||||||
|
**Constants:**
|
||||||
|
- `UNMATCHED_PENALTY = 10,000` - Cost per unmatched child (create/delete is categorically more expensive than property tweaks)
|
||||||
|
- `MAX_SCORING_DEPTH = 3` - Depth limit for recursive scoring to prevent O(n^k) explosion
|
||||||
|
|
||||||
|
**`compute_change_count()` and `match_children()` are mutually recursive.** Base case: leaf nodes (no children).
|
||||||
|
|
||||||
|
**Output:** `MatchResult { matched: Vec<(new_ref, old_ref)>, unmatched_new: Vec<new_ref>, unmatched_old: Vec<old_ref> }`
|
||||||
|
|
||||||
|
**Known limitation:** Greedy assignment is not globally optimal. The Hungarian algorithm (O(N^3)) would give optimal results, but ambiguous groups are typically small (2-10 instances).
|
||||||
|
|
||||||
|
## Dedup Suffix Management (`dedup_suffix.rs`)
|
||||||
|
|
||||||
|
Manages `~N` suffix cleanup after instance deletions during two-way sync.
|
||||||
|
|
||||||
|
**Key functions:**
|
||||||
|
- `parse_dedup_suffix(stem) -> Option<(base, N)>` - Parse `~N` from filename stem
|
||||||
|
- `build_dedup_name(base, suffix, ext)` - Build filename with optional `~N` suffix
|
||||||
|
- `compute_cleanup_action(removed_stem, remaining_stems) -> DedupCleanupAction` - Determine what to do after a deletion
|
||||||
|
|
||||||
|
**Cleanup rules:**
|
||||||
|
- **Gap-tolerant:** Deleting `~1` from `{Foo, Foo~1, Foo~2}` leaves `{Foo, Foo~2}`. No renumbering.
|
||||||
|
- **Base-name promotion:** Deleting `Foo` (base) from `{Foo, Foo~1, Foo~2}` promotes `Foo~1` to `Foo`. `Foo~2` stays.
|
||||||
|
- **Group-to-1 cleanup:** If group shrinks to 1 member, remove its suffix entirely.
|
||||||
|
|
||||||
## Syncback Rules
|
## Syncback Rules
|
||||||
|
|
||||||
Configured in `syncbackRules` section of project file:
|
Configured in `syncbackRules` section of project file:
|
||||||
|
|
@ -246,12 +286,13 @@ snapshot.with_new_path(path, new_ref, old_ref)
|
||||||
```
|
```
|
||||||
|
|
||||||
**Filename generation** (`file_names.rs`):
|
**Filename generation** (`file_names.rs`):
|
||||||
- Slugifies instance name (`slugify_name()`: forbidden chars → `_`)
|
- `slugify_name()`: forbidden chars (`<>:"/\|?*~`) → `_`. Also strips leading/trailing spaces/dots, neutralizes dangerous suffixes (`.server`, `.meta`, etc.), handles Windows reserved names (`CON`, `PRN`, etc.)
|
||||||
- Deduplicates against taken names (`deduplicate_name()`: appends `~1`, `~2`, etc.)
|
- `deduplicate_name_with_ext()`: Dedup key = full filesystem name (slug + extension), case-insensitive. `Folder "Foo"` (key `"foo"`) and `Script "Foo"` (key `"foo.server.luau"`) do NOT collide. Appends `~1`, `~2`, etc. on collision, with suffix inserted before compound extension: `Foo~1.server.luau`
|
||||||
- Adds appropriate extension based on middleware
|
- `name_for_inst()` returns `(filename, needs_meta_name, dedup_key)`. `needs_meta_name` is true when the filesystem name differs from the real instance name.
|
||||||
- Stores real name in `.meta.json5` `name` field when slug differs
|
- `~` is in `SLUGIFY_CHARS` so natural tilde names (e.g., `"Foo~1"`) get slugified to `"Foo_1"`, avoiding confusion with dedup suffixes.
|
||||||
- Example: `Script` named "My Script" → `My Script.server.luau`
|
- Example: `Script` named "My Script" → `My Script.server.luau`
|
||||||
- Example: `Script` named "Hey/Bro" → `Hey_Bro.server.luau` + meta with `name: "Hey/Bro"`
|
- Example: `Script` named "Hey/Bro" → `Hey_Bro.server.luau` + meta with `name: "Hey/Bro"`
|
||||||
|
- Example: Two `Folder "Data"` → `Data/` + `Data~1/` (meta: `name: "Data"`)
|
||||||
|
|
||||||
### Property Filtering
|
### Property Filtering
|
||||||
|
|
||||||
|
|
@ -301,6 +342,10 @@ fs_snapshot.merge(other) // Combine snapshots
|
||||||
|
|
||||||
// Apply to filesystem
|
// Apply to filesystem
|
||||||
fs_snapshot.write_to_vfs(base_path, vfs)?;
|
fs_snapshot.write_to_vfs(base_path, vfs)?;
|
||||||
|
fs_snapshot.write_to_vfs_parallel(base_path, vfs)?; // Parallel writes with retry logic
|
||||||
|
|
||||||
|
// Post-process ref paths
|
||||||
|
fs_snapshot.fix_ref_paths(&substitutions); // Replace tentative paths with final dedup'd paths
|
||||||
|
|
||||||
// Query
|
// Query
|
||||||
fs_snapshot.is_empty()
|
fs_snapshot.is_empty()
|
||||||
|
|
@ -310,26 +355,53 @@ fs_snapshot.removed_paths()
|
||||||
|
|
||||||
## Ref Properties (`ref_properties.rs`)
|
## Ref Properties (`ref_properties.rs`)
|
||||||
|
|
||||||
**Challenge:** Ref properties (like `ObjectValue.Value`) point to other instances by Ref, but Refs are different between old and new trees.
|
**Challenge:** Ref properties (like `ObjectValue.Value`, `Model.PrimaryPart`) point to other instances by Ref, but Refs are different between old and new trees.
|
||||||
|
|
||||||
**Solution:**
|
**Solution: Path-based `Rojo_Ref_*` attributes (preferred)**
|
||||||
|
|
||||||
1. **Collect paths** - Map every Ref → full instance path (e.g., `Workspace.Part1`)
|
Syncback writes filesystem-name-based paths as `Rojo_Ref_PropertyName` attributes:
|
||||||
2. **Collect referents** - Find all properties that are Refs, store as (source_path, target_path)
|
|
||||||
3. **Link referents** - After syncback, use Atlas's `$id` / `Rojo_Target_PropertyName` system to link instances
|
|
||||||
|
|
||||||
**Atlas linking system:**
|
```json5
|
||||||
```json
|
|
||||||
{
|
{
|
||||||
"id": "some-unique-id",
|
|
||||||
"properties": {},
|
"properties": {},
|
||||||
"attributes": {
|
"attributes": {
|
||||||
"Rojo_Target_Value": "some-unique-id"
|
"Rojo_Ref_PrimaryPart": "Workspace/Handle.model.json5"
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
```
|
```
|
||||||
|
|
||||||
Plugin reads `Rojo_Target_PropertyName` attributes and sets corresponding Ref properties.
|
Forward sync reads `Rojo_Ref_*` attributes and resolves them to tree instances via `get_instance_by_path()`.
|
||||||
|
|
||||||
|
**Legacy: `Rojo_Target_*` / `$id` system (still supported for reading)**
|
||||||
|
|
||||||
|
```json5
|
||||||
|
{
|
||||||
|
"id": "some-unique-id",
|
||||||
|
"attributes": { "Rojo_Target_Value": "some-unique-id" }
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Forward sync also reads `Rojo_Target_*` attributes and resolves via `get_specified_id()`. Priority: `Rojo_Ref_*` takes precedence over `Rojo_Target_*`.
|
||||||
|
|
||||||
|
**Key functions:**
|
||||||
|
- `collect_all_paths(dom)` - Maps all Refs to filesystem-name-compatible paths (called pre-prune for external refs)
|
||||||
|
- `collect_referents(dom, paths, final_paths)` - Finds all Ref properties, stores as (source, target_path) pairs
|
||||||
|
- `tentative_fs_path(dom, ref)` - Computes pre-dedup path: slugified name + extension based on class/children
|
||||||
|
- `tentative_fs_path_public()` - Public wrapper used in post-processing
|
||||||
|
- `link_referents(dom, referents)` - Writes `Rojo_Ref_*` attributes to instances
|
||||||
|
|
||||||
|
**Ref path post-processing (in `mod.rs`):**
|
||||||
|
|
||||||
|
During the syncback walk, `record_ref_path()` records each instance's final dedup'd filesystem path in `ref_path_map`. After the walk, `fix_ref_paths()` compares tentative paths to final paths and substitutes in all written meta/model files. Substitutions are sorted by length descending to prevent partial prefix matches.
|
||||||
|
|
||||||
|
## Meta File Helpers (`meta.rs`)
|
||||||
|
|
||||||
|
Shared helpers for reading/writing `.meta.json5` and `.model.json5` files:
|
||||||
|
|
||||||
|
- `upsert_meta_name(path, name)` - Upsert `name` field in `.meta.json5`
|
||||||
|
- `remove_meta_name(path)` - Remove `name` field; returns `RemoveNameOutcome` (NoOp, FieldRemoved, FileDeleted)
|
||||||
|
- `upsert_model_name(path, name)` - Upsert `name` field in `.model.json5`
|
||||||
|
- `update_ref_paths_in_file(path, old_prefix, new_prefix)` - Update `Rojo_Ref_*` attribute paths in a meta/model file
|
||||||
|
|
||||||
## Statistics Tracking (`stats.rs`)
|
## Statistics Tracking (`stats.rs`)
|
||||||
|
|
||||||
|
|
@ -433,7 +505,7 @@ Filesystem:
|
||||||
2. Sending file to server
|
2. Sending file to server
|
||||||
3. Server runs syncback command
|
3. Server runs syncback command
|
||||||
|
|
||||||
**Two-way sync (experimental)** is different - see `atlas-api.mdc`.
|
**Two-way sync** is different - see `atlas-api.mdc`.
|
||||||
|
|
||||||
## Performance Considerations
|
## Performance Considerations
|
||||||
|
|
||||||
|
|
@ -441,7 +513,7 @@ Filesystem:
|
||||||
|
|
||||||
**Incremental benefit:** On large projects, incremental mode with hashing can be 10-100x faster than rewriting everything.
|
**Incremental benefit:** On large projects, incremental mode with hashing can be 10-100x faster than rewriting everything.
|
||||||
|
|
||||||
**Parallelization:** Not currently parallelized - filesystem writes are sequential.
|
**Parallelization:** File writes can be parallelized via `write_to_vfs_parallel()` (includes Windows retry logic).
|
||||||
|
|
||||||
## Debugging
|
## Debugging
|
||||||
|
|
||||||
|
|
@ -473,14 +545,20 @@ diff place.rbxl rebuilt.rbxl
|
||||||
**From code comments:**
|
**From code comments:**
|
||||||
- Better detection of when root is directory vs file
|
- Better detection of when root is directory vs file
|
||||||
- More sophisticated middleware selection
|
- More sophisticated middleware selection
|
||||||
- Parallel file writes
|
|
||||||
- Better handling of Roblox format changes
|
- Better handling of Roblox format changes
|
||||||
- Support for partial syncback (only changed instances)
|
- Support for partial syncback (only changed instances)
|
||||||
|
- Globally optimal matching (Hungarian algorithm) for large ambiguous groups
|
||||||
|
- Matching cache (parent_ref -> match_assignments) for avoiding redundant scoring
|
||||||
|
|
||||||
## Key Files to Reference
|
## Key Files to Reference
|
||||||
|
|
||||||
- `src/syncback/mod.rs` - Main orchestration loop
|
- `src/syncback/mod.rs` - Main orchestration loop, ref path post-processing
|
||||||
- `src/syncback/snapshot.rs` - SyncbackSnapshot implementation
|
- `src/syncback/snapshot.rs` - SyncbackSnapshot implementation
|
||||||
|
- `src/syncback/matching.rs` - Recursive change-count matching algorithm
|
||||||
|
- `src/syncback/file_names.rs` - slugify_name(), deduplicate_name_with_ext()
|
||||||
|
- `src/syncback/dedup_suffix.rs` - Dedup cleanup rules
|
||||||
|
- `src/syncback/meta.rs` - Meta file read/write helpers
|
||||||
|
- `src/syncback/ref_properties.rs` - Ref property path linking
|
||||||
- `src/syncback/hash/` - Hashing algorithm
|
- `src/syncback/hash/` - Hashing algorithm
|
||||||
- `src/syncback/property_filter.rs` - Property filtering logic
|
- `src/syncback/property_filter.rs` - Property filtering logic
|
||||||
- `src/cli/syncback.rs` - CLI command implementation
|
- `src/cli/syncback.rs` - CLI command implementation
|
||||||
|
|
|
||||||
|
|
@ -41,7 +41,7 @@ Atlas bridges filesystem and Roblox Studio, enabling professional development to
|
||||||
**Current state:**
|
**Current state:**
|
||||||
- `atlas syncback` (CLI) is the **ground truth** for Roblox → filesystem syncing. It is battle-tested and handles edge cases reliably.
|
- `atlas syncback` (CLI) is the **ground truth** for Roblox → filesystem syncing. It is battle-tested and handles edge cases reliably.
|
||||||
- Plugin-based two-way sync (via `atlas serve` + the Studio plugin) is under active development to reach feature parity and match the resilience of CLI syncback.
|
- Plugin-based two-way sync (via `atlas serve` + the Studio plugin) is under active development to reach feature parity and match the resilience of CLI syncback.
|
||||||
- The bar for replacing syncback is high: plugin two-way sync must handle all the same scenarios (encoded names, nested projects, property conflicts, large trees, concurrent edits) just as reliably before syncback can be deprecated.
|
- The bar for replacing syncback is high: plugin two-way sync must handle all the same scenarios (slugified names, dedup suffixes, nested projects, property conflicts, large trees, concurrent edits) just as reliably before syncback can be deprecated.
|
||||||
|
|
||||||
**When working on two-way sync:** Plugin-based sync must produce **exactly the same filesystem output** that `atlas syncback` would give for the same input. Byte-for-byte identical files, identical directory structures, identical naming. If the plugin path produces different results, treat syncback as the correct reference unless there's an explicit, documented reason to diverge. Any divergence is a bug until proven otherwise.
|
**When working on two-way sync:** Plugin-based sync must produce **exactly the same filesystem output** that `atlas syncback` would give for the same input. Byte-for-byte identical files, identical directory structures, identical naming. If the plugin path produces different results, treat syncback as the correct reference unless there's an explicit, documented reason to diverge. Any divergence is a bug until proven otherwise.
|
||||||
|
|
||||||
|
|
@ -316,16 +316,25 @@ Reconciler.applyPatch(instanceMap, patch)
|
||||||
src/ - Rust server/CLI
|
src/ - Rust server/CLI
|
||||||
cli/ - Command implementations
|
cli/ - Command implementations
|
||||||
snapshot/ - Core snapshot system
|
snapshot/ - Core snapshot system
|
||||||
|
matching.rs - Forward sync matching algorithm
|
||||||
snapshot_middleware/ - File type handlers
|
snapshot_middleware/ - File type handlers
|
||||||
syncback/ - Roblox → filesystem
|
syncback/ - Roblox → filesystem
|
||||||
|
matching.rs - Syncback matching algorithm
|
||||||
|
dedup_suffix.rs - Dedup cleanup rules
|
||||||
|
meta.rs - Meta file read/write helpers
|
||||||
web/ - HTTP/WebSocket server
|
web/ - HTTP/WebSocket server
|
||||||
change_processor.rs - Two-way sync filesystem writer
|
change_processor.rs - Two-way sync filesystem writer
|
||||||
|
rojo_ref.rs - Ref path system (Rojo_Ref_*, RefPathIndex)
|
||||||
|
variant_eq.rs - Property value comparison
|
||||||
|
|
||||||
plugin/src/ - Lua Studio plugin
|
plugin/src/ - Lua Studio plugin
|
||||||
App/ - Roact UI components
|
App/ - Roact UI components
|
||||||
Reconciler/ - Patch application logic
|
Reconciler/ - Patch application logic
|
||||||
|
matching.lua - Lua matching algorithm
|
||||||
|
trueEquals.lua - Shared value equality module
|
||||||
ChangeBatcher/ - Two-way sync patch creation
|
ChangeBatcher/ - Two-way sync patch creation
|
||||||
Config.lua - Version and protocol config
|
Config.lua - Version and protocol config
|
||||||
|
XXH32.luau - Hash function for plugin
|
||||||
|
|
||||||
rbx-dom/ - Git submodule (separate repo)
|
rbx-dom/ - Git submodule (separate repo)
|
||||||
rbx_binary/ - Binary format (rbxm/rbxl)
|
rbx_binary/ - Binary format (rbxm/rbxl)
|
||||||
|
|
|
||||||
|
|
@ -1074,6 +1074,15 @@ impl JobThreadContext {
|
||||||
// groups need suffix renames (base-name promotion, group-to-1).
|
// groups need suffix renames (base-name promotion, group-to-1).
|
||||||
// Must run BEFORE apply_patch_set removes the instances from
|
// Must run BEFORE apply_patch_set removes the instances from
|
||||||
// the tree, because we need parent/sibling relationships.
|
// the tree, because we need parent/sibling relationships.
|
||||||
|
//
|
||||||
|
// Build a set of ALL removed Refs so the sibling enumeration
|
||||||
|
// skips co-removed instances (Fix 2: batch-removal awareness).
|
||||||
|
let removed_set: std::collections::HashSet<Ref> =
|
||||||
|
patch_set.removed_instances.iter().copied().collect();
|
||||||
|
// Collect metadata updates for survivors whose filesystem path
|
||||||
|
// changed due to dedup cleanup renames (Fix 1: metadata drift).
|
||||||
|
let mut dedup_metadata_updates: Vec<(Ref, PathBuf)> = Vec::new();
|
||||||
|
|
||||||
for &removed_id in &patch_set.removed_instances {
|
for &removed_id in &patch_set.removed_instances {
|
||||||
let (parent_ref, removed_fs_name) = {
|
let (parent_ref, removed_fs_name) = {
|
||||||
let Some(inst) = tree.get_instance(removed_id) else {
|
let Some(inst) = tree.get_instance(removed_id) else {
|
||||||
|
|
@ -1118,7 +1127,10 @@ impl JobThreadContext {
|
||||||
let deleted_was_base = parse_dedup_suffix(removed_stem).is_none();
|
let deleted_was_base = parse_dedup_suffix(removed_stem).is_none();
|
||||||
|
|
||||||
for &sibling_ref in parent_inst.children() {
|
for &sibling_ref in parent_inst.children() {
|
||||||
if sibling_ref == removed_id {
|
// Skip ALL instances being removed in this patch, not just
|
||||||
|
// the current one. This prevents co-removed siblings from
|
||||||
|
// inflating the remaining count and defeating cleanup rules.
|
||||||
|
if removed_set.contains(&sibling_ref) {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
let sibling_fs = tree.filesystem_name_for(sibling_ref);
|
let sibling_fs = tree.filesystem_name_for(sibling_ref);
|
||||||
|
|
@ -1190,13 +1202,58 @@ impl JobThreadContext {
|
||||||
let new_ref_segment =
|
let new_ref_segment =
|
||||||
to.file_name().and_then(|f| f.to_str()).unwrap_or("");
|
to.file_name().and_then(|f| f.to_str()).unwrap_or("");
|
||||||
if old_ref_segment != new_ref_segment {
|
if old_ref_segment != new_ref_segment {
|
||||||
// Build the old and new ref path prefixes
|
|
||||||
let parent_path =
|
let parent_path =
|
||||||
crate::ref_target_path_from_tree(&tree, parent_ref);
|
crate::ref_target_path_from_tree(&tree, parent_ref);
|
||||||
let old_prefix = format!("{}/{}", parent_path, old_ref_segment);
|
let old_prefix = format!("{}/{}", parent_path, old_ref_segment);
|
||||||
let new_prefix = format!("{}/{}", parent_path, new_ref_segment);
|
let new_prefix = format!("{}/{}", parent_path, new_ref_segment);
|
||||||
self.update_ref_paths_after_rename(&old_prefix, &new_prefix);
|
self.update_ref_paths_after_rename(&old_prefix, &new_prefix);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Fix 1: Update the renamed survivor's in-memory
|
||||||
|
// metadata so InstigatingSource::Path and
|
||||||
|
// relevant_paths point to the new filesystem path.
|
||||||
|
let old_from_name =
|
||||||
|
from.file_name().and_then(|f| f.to_str()).unwrap_or("");
|
||||||
|
for &sibling_ref in parent_inst.children() {
|
||||||
|
if removed_set.contains(&sibling_ref) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
let sibling_fs = tree.filesystem_name_for(sibling_ref);
|
||||||
|
if sibling_fs == old_from_name {
|
||||||
|
// This is the survivor being renamed.
|
||||||
|
// Compute its new instigating source path.
|
||||||
|
if let Some(meta) = tree.get_metadata(sibling_ref) {
|
||||||
|
if let Some(InstigatingSource::Path(old_path)) =
|
||||||
|
&meta.instigating_source
|
||||||
|
{
|
||||||
|
let new_source = if old_path
|
||||||
|
.file_name()
|
||||||
|
.and_then(|f| f.to_str())
|
||||||
|
.map(|f| f.starts_with("init."))
|
||||||
|
.unwrap_or(false)
|
||||||
|
{
|
||||||
|
// Directory-format: old source
|
||||||
|
// is dir/init.luau; the dir was
|
||||||
|
// renamed so update the dir
|
||||||
|
// portion.
|
||||||
|
let init_name = old_path
|
||||||
|
.file_name()
|
||||||
|
.unwrap()
|
||||||
|
.to_str()
|
||||||
|
.unwrap();
|
||||||
|
to.join(init_name)
|
||||||
|
} else {
|
||||||
|
// Standalone file: new source
|
||||||
|
// IS the `to` path.
|
||||||
|
to.clone()
|
||||||
|
};
|
||||||
|
dedup_metadata_updates
|
||||||
|
.push((sibling_ref, new_source));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -1778,7 +1835,9 @@ impl JobThreadContext {
|
||||||
// This keeps instigating_source and path_to_ids in sync so that
|
// This keeps instigating_source and path_to_ids in sync so that
|
||||||
// subsequent VFS events (and future renames) target the correct
|
// subsequent VFS events (and future renames) target the correct
|
||||||
// instance and path.
|
// instance and path.
|
||||||
for (id, new_instigating_source) in metadata_updates {
|
for (id, new_instigating_source) in
|
||||||
|
metadata_updates.into_iter().chain(dedup_metadata_updates)
|
||||||
|
{
|
||||||
if let Some(old_metadata) = tree.get_metadata(id).cloned() {
|
if let Some(old_metadata) = tree.get_metadata(id).cloned() {
|
||||||
let mut new_metadata = old_metadata;
|
let mut new_metadata = old_metadata;
|
||||||
new_metadata.instigating_source =
|
new_metadata.instigating_source =
|
||||||
|
|
|
||||||
|
|
@ -451,8 +451,10 @@ impl FsSnapshot {
|
||||||
///
|
///
|
||||||
/// For each entry in `substitutions` (old_path → new_path), finds all
|
/// For each entry in `substitutions` (old_path → new_path), finds all
|
||||||
/// `.meta.json5` and `.model.json5` files in the snapshot and replaces
|
/// `.meta.json5` and `.model.json5` files in the snapshot and replaces
|
||||||
/// occurrences of the old path with the new path. This fixes tentative
|
/// occurrences of the old path with the new path **only on lines that
|
||||||
/// ref paths that didn't include dedup suffixes.
|
/// contain a `Rojo_Ref_` key**. This prevents unrelated string values
|
||||||
|
/// (property values, comments, etc.) from being altered by the
|
||||||
|
/// substitution.
|
||||||
pub fn fix_ref_paths(&mut self, substitutions: &[(String, String)]) {
|
pub fn fix_ref_paths(&mut self, substitutions: &[(String, String)]) {
|
||||||
if substitutions.is_empty() {
|
if substitutions.is_empty() {
|
||||||
return;
|
return;
|
||||||
|
|
@ -482,18 +484,95 @@ impl FsSnapshot {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
let mut modified = text.to_string();
|
// Attribute-scoped replacement: only apply substitutions on
|
||||||
for (old_path, new_path) in substitutions {
|
// lines that contain a Rojo_Ref_ key. This prevents accidental
|
||||||
if old_path == new_path {
|
// mutation of unrelated string values in the same file.
|
||||||
continue;
|
let mut modified_lines: Vec<String> = Vec::new();
|
||||||
|
let mut any_changed = false;
|
||||||
|
for line in text.split('\n') {
|
||||||
|
if line.contains("Rojo_Ref_") {
|
||||||
|
let mut new_line = line.to_string();
|
||||||
|
for (old_path, new_path) in substitutions {
|
||||||
|
if old_path == new_path {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
new_line = new_line.replace(old_path.as_str(), new_path.as_str());
|
||||||
|
}
|
||||||
|
if new_line != line {
|
||||||
|
any_changed = true;
|
||||||
|
}
|
||||||
|
modified_lines.push(new_line);
|
||||||
|
} else {
|
||||||
|
modified_lines.push(line.to_string());
|
||||||
}
|
}
|
||||||
// Replace exact path matches and prefix matches (path + "/")
|
|
||||||
modified = modified.replace(old_path.as_str(), new_path.as_str());
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if modified != text {
|
if any_changed {
|
||||||
*contents = modified.into_bytes();
|
*contents = modified_lines.join("\n").into_bytes();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn fix_ref_paths_only_touches_ref_lines() {
|
||||||
|
let mut snap = FsSnapshot::new();
|
||||||
|
|
||||||
|
let meta_content = r#"{
|
||||||
|
"properties": {
|
||||||
|
"Description": "Path is Workspace/Foo"
|
||||||
|
},
|
||||||
|
"attributes": {
|
||||||
|
"Rojo_Ref_PrimaryPart": "Workspace/Foo"
|
||||||
|
}
|
||||||
|
}"#;
|
||||||
|
snap.added_files.insert(
|
||||||
|
PathBuf::from("/test/init.meta.json5"),
|
||||||
|
meta_content.as_bytes().to_vec(),
|
||||||
|
);
|
||||||
|
|
||||||
|
let substitutions = vec![("Workspace/Foo".to_string(), "Workspace/Foo~1".to_string())];
|
||||||
|
snap.fix_ref_paths(&substitutions);
|
||||||
|
|
||||||
|
let result = std::str::from_utf8(snap.added_files.get(Path::new("/test/init.meta.json5")).unwrap()).unwrap();
|
||||||
|
|
||||||
|
assert!(
|
||||||
|
result.contains(r#""Rojo_Ref_PrimaryPart": "Workspace/Foo~1""#),
|
||||||
|
"Rojo_Ref line should be updated. Got:\n{}",
|
||||||
|
result
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
result.contains(r#""Description": "Path is Workspace/Foo""#),
|
||||||
|
"Non-ref property line should NOT be changed. Got:\n{}",
|
||||||
|
result
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn fix_ref_paths_ignores_non_meta_files() {
|
||||||
|
let mut snap = FsSnapshot::new();
|
||||||
|
|
||||||
|
snap.added_files.insert(
|
||||||
|
PathBuf::from("/test/script.luau"),
|
||||||
|
b"-- Rojo_Ref_PrimaryPart Workspace/Foo".to_vec(),
|
||||||
|
);
|
||||||
|
|
||||||
|
let substitutions = vec![("Workspace/Foo".to_string(), "Workspace/Foo~1".to_string())];
|
||||||
|
snap.fix_ref_paths(&substitutions);
|
||||||
|
|
||||||
|
let result = std::str::from_utf8(snap.added_files.get(Path::new("/test/script.luau")).unwrap()).unwrap();
|
||||||
|
assert!(
|
||||||
|
result.contains("Workspace/Foo"),
|
||||||
|
"Non-meta file should not be touched. Got: {}",
|
||||||
|
result
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
!result.contains("Workspace/Foo~1"),
|
||||||
|
"Non-meta file should not have substitution applied"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -619,6 +619,9 @@ impl ApiService {
|
||||||
// Build a path map for instances added in this same request.
|
// Build a path map for instances added in this same request.
|
||||||
// This allows Ref properties in updates to reference just-added
|
// This allows Ref properties in updates to reference just-added
|
||||||
// instances that aren't in the tree yet (VFS watcher is async).
|
// instances that aren't in the tree yet (VFS watcher is async).
|
||||||
|
// The segment includes the middleware extension (e.g. .server.luau,
|
||||||
|
// .model.json5) so that Rojo_Ref_* paths match what the filesystem
|
||||||
|
// will actually contain.
|
||||||
let added_paths: HashMap<Ref, String> = {
|
let added_paths: HashMap<Ref, String> = {
|
||||||
let tree = self.serve_session.tree();
|
let tree = self.serve_session.tree();
|
||||||
request
|
request
|
||||||
|
|
@ -631,14 +634,7 @@ impl ApiService {
|
||||||
} else {
|
} else {
|
||||||
return None;
|
return None;
|
||||||
};
|
};
|
||||||
// Use slugified name (matching filesystem conventions) instead of
|
let fs_name = added_instance_fs_segment(added);
|
||||||
// escape_ref_path_segment (which uses legacy \/ escaping that
|
|
||||||
// get_instance_by_path cannot resolve).
|
|
||||||
let fs_name = if crate::syncback::name_needs_slugify(&added.name) {
|
|
||||||
crate::syncback::slugify_name(&added.name)
|
|
||||||
} else {
|
|
||||||
added.name.clone()
|
|
||||||
};
|
|
||||||
let path = if parent_path.is_empty() {
|
let path = if parent_path.is_empty() {
|
||||||
fs_name
|
fs_name
|
||||||
} else {
|
} else {
|
||||||
|
|
@ -3562,6 +3558,91 @@ fn instance_for_scripts_only<'a>(
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Compute the tentative filesystem name segment for an `AddedInstance`.
|
||||||
|
///
|
||||||
|
/// Mirrors the logic of `tentative_fs_name` in `ref_properties.rs` but
|
||||||
|
/// operates on `AddedInstance` fields instead of `rbx_dom_weak::Instance`.
|
||||||
|
/// Used by `added_paths` to build correct same-batch Ref target paths.
|
||||||
|
fn added_instance_fs_segment(added: &crate::web::interface::AddedInstance) -> String {
|
||||||
|
let slug = if crate::syncback::name_needs_slugify(&added.name) {
|
||||||
|
crate::syncback::slugify_name(&added.name)
|
||||||
|
} else {
|
||||||
|
added.name.clone()
|
||||||
|
};
|
||||||
|
|
||||||
|
let has_children = !added.children.is_empty();
|
||||||
|
|
||||||
|
let is_container = matches!(
|
||||||
|
added.class_name.as_str(),
|
||||||
|
"Folder"
|
||||||
|
| "Configuration"
|
||||||
|
| "Tool"
|
||||||
|
| "ScreenGui"
|
||||||
|
| "SurfaceGui"
|
||||||
|
| "BillboardGui"
|
||||||
|
| "AdGui"
|
||||||
|
);
|
||||||
|
|
||||||
|
let is_script = matches!(
|
||||||
|
added.class_name.as_str(),
|
||||||
|
"Script" | "LocalScript" | "ModuleScript"
|
||||||
|
);
|
||||||
|
|
||||||
|
if is_container || (has_children && is_script) {
|
||||||
|
return slug;
|
||||||
|
}
|
||||||
|
|
||||||
|
let extension = match added.class_name.as_str() {
|
||||||
|
"Script" => {
|
||||||
|
if has_children {
|
||||||
|
return slug;
|
||||||
|
}
|
||||||
|
match added.properties.get("RunContext") {
|
||||||
|
Some(Variant::Enum(e)) => match e.to_u32() {
|
||||||
|
0 => "legacy.luau",
|
||||||
|
1 => "server.luau",
|
||||||
|
2 => "client.luau",
|
||||||
|
3 => "plugin.luau",
|
||||||
|
_ => "legacy.luau",
|
||||||
|
},
|
||||||
|
_ => "legacy.luau",
|
||||||
|
}
|
||||||
|
}
|
||||||
|
"LocalScript" => {
|
||||||
|
if has_children {
|
||||||
|
return slug;
|
||||||
|
}
|
||||||
|
"local.luau"
|
||||||
|
}
|
||||||
|
"ModuleScript" => {
|
||||||
|
if has_children {
|
||||||
|
return slug;
|
||||||
|
}
|
||||||
|
"luau"
|
||||||
|
}
|
||||||
|
"StringValue" => {
|
||||||
|
if has_children {
|
||||||
|
return slug;
|
||||||
|
}
|
||||||
|
"txt"
|
||||||
|
}
|
||||||
|
"LocalizationTable" => {
|
||||||
|
if has_children {
|
||||||
|
return slug;
|
||||||
|
}
|
||||||
|
"csv"
|
||||||
|
}
|
||||||
|
_ => {
|
||||||
|
if has_children {
|
||||||
|
return slug;
|
||||||
|
}
|
||||||
|
"model.json5"
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
format!("{slug}.{extension}")
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
|
|
|
||||||
|
|
@ -6764,13 +6764,14 @@ fn ref_to_instance_added_in_same_request() {
|
||||||
|
|
||||||
// The meta file should contain Rojo_Ref_PrimaryPart pointing to the new Part.
|
// The meta file should contain Rojo_Ref_PrimaryPart pointing to the new Part.
|
||||||
// The added_paths fallback resolves the path from the AddedInstance data.
|
// The added_paths fallback resolves the path from the AddedInstance data.
|
||||||
|
// The path includes the middleware extension (Fix 3: same-batch add+ref).
|
||||||
let meta_path = session
|
let meta_path = session
|
||||||
.path()
|
.path()
|
||||||
.join("src/Workspace/TestModel/init.meta.json5");
|
.join("src/Workspace/TestModel/init.meta.json5");
|
||||||
assert_meta_has_ref_attr(
|
assert_meta_has_ref_attr(
|
||||||
&meta_path,
|
&meta_path,
|
||||||
"Rojo_Ref_PrimaryPart",
|
"Rojo_Ref_PrimaryPart",
|
||||||
"Workspace/TestModel/NewPart",
|
"Workspace/TestModel/NewPart.model.json5",
|
||||||
);
|
);
|
||||||
|
|
||||||
// Verify the tree also has a resolved PrimaryPart Ref (not dangling).
|
// Verify the tree also has a resolved PrimaryPart Ref (not dangling).
|
||||||
|
|
@ -7215,3 +7216,168 @@ fn ref_on_txt_file_instance() {
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// =========================================================================
|
||||||
|
// Regression tests for audit_ambiguous_paths_followup fixes
|
||||||
|
// =========================================================================
|
||||||
|
|
||||||
|
/// Fix 3 regression: when adding a new Script instance and a Ref property
|
||||||
|
/// pointing to it in the same /api/write batch, the persisted Rojo_Ref_*
|
||||||
|
/// path must include the middleware extension (e.g. ".server.luau").
|
||||||
|
#[test]
|
||||||
|
fn ref_same_batch_add_target_includes_extension() {
|
||||||
|
run_serve_test("ref_two_way_sync", |session, _| {
|
||||||
|
let (session_id, workspace_id, model_id, _, _, _) = ref_test_setup(&session);
|
||||||
|
|
||||||
|
// Add a new Script as a child of Workspace
|
||||||
|
let target_guid = Ref::new();
|
||||||
|
let mut add_props = HashMap::new();
|
||||||
|
add_props.insert(
|
||||||
|
"Source".to_string(),
|
||||||
|
Variant::String("-- target script".to_string()),
|
||||||
|
);
|
||||||
|
add_props.insert(
|
||||||
|
"RunContext".to_string(),
|
||||||
|
Variant::Enum(rbx_dom_weak::types::Enum::from_u32(1)), // Server
|
||||||
|
);
|
||||||
|
|
||||||
|
let added_inst = AddedInstance {
|
||||||
|
parent: Some(workspace_id),
|
||||||
|
name: "RefTarget".to_string(),
|
||||||
|
class_name: "Script".to_string(),
|
||||||
|
properties: add_props,
|
||||||
|
children: vec![],
|
||||||
|
};
|
||||||
|
let mut added_map = HashMap::new();
|
||||||
|
added_map.insert(target_guid, added_inst);
|
||||||
|
|
||||||
|
// In the same request, set PrimaryPart on TestModel to the new Script
|
||||||
|
let mut update_props = UstrMap::default();
|
||||||
|
update_props.insert(ustr("PrimaryPart"), Some(Variant::Ref(target_guid)));
|
||||||
|
|
||||||
|
let write_request = WriteRequest {
|
||||||
|
session_id,
|
||||||
|
removed: vec![],
|
||||||
|
added: added_map,
|
||||||
|
updated: vec![InstanceUpdate {
|
||||||
|
id: model_id,
|
||||||
|
changed_name: None,
|
||||||
|
changed_class_name: None,
|
||||||
|
changed_properties: update_props,
|
||||||
|
changed_metadata: None,
|
||||||
|
}],
|
||||||
|
};
|
||||||
|
session.post_api_write(&write_request).unwrap();
|
||||||
|
thread::sleep(Duration::from_millis(500));
|
||||||
|
|
||||||
|
// The Rojo_Ref_PrimaryPart path should include the extension
|
||||||
|
let meta_path = session
|
||||||
|
.path()
|
||||||
|
.join("src/Workspace/TestModel/init.meta.json5");
|
||||||
|
poll_meta_has_ref_attr(
|
||||||
|
&meta_path,
|
||||||
|
"Rojo_Ref_PrimaryPart",
|
||||||
|
"Workspace/RefTarget.server.luau",
|
||||||
|
);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Fix 3 regression: same-batch add+ref for a ModuleScript target should
|
||||||
|
/// produce a path ending in ".luau" (not bare instance name).
|
||||||
|
#[test]
|
||||||
|
fn ref_same_batch_add_module_target_includes_extension() {
|
||||||
|
run_serve_test("ref_two_way_sync", |session, _| {
|
||||||
|
let (session_id, workspace_id, model_id, _, _, _) = ref_test_setup(&session);
|
||||||
|
|
||||||
|
let target_guid = Ref::new();
|
||||||
|
let mut add_props = HashMap::new();
|
||||||
|
add_props.insert(
|
||||||
|
"Source".to_string(),
|
||||||
|
Variant::String("return {}".to_string()),
|
||||||
|
);
|
||||||
|
|
||||||
|
let added_inst = AddedInstance {
|
||||||
|
parent: Some(workspace_id),
|
||||||
|
name: "ModuleTarget".to_string(),
|
||||||
|
class_name: "ModuleScript".to_string(),
|
||||||
|
properties: add_props,
|
||||||
|
children: vec![],
|
||||||
|
};
|
||||||
|
let mut added_map = HashMap::new();
|
||||||
|
added_map.insert(target_guid, added_inst);
|
||||||
|
|
||||||
|
let mut update_props = UstrMap::default();
|
||||||
|
update_props.insert(ustr("PrimaryPart"), Some(Variant::Ref(target_guid)));
|
||||||
|
|
||||||
|
let write_request = WriteRequest {
|
||||||
|
session_id,
|
||||||
|
removed: vec![],
|
||||||
|
added: added_map,
|
||||||
|
updated: vec![InstanceUpdate {
|
||||||
|
id: model_id,
|
||||||
|
changed_name: None,
|
||||||
|
changed_class_name: None,
|
||||||
|
changed_properties: update_props,
|
||||||
|
changed_metadata: None,
|
||||||
|
}],
|
||||||
|
};
|
||||||
|
session.post_api_write(&write_request).unwrap();
|
||||||
|
thread::sleep(Duration::from_millis(500));
|
||||||
|
|
||||||
|
let meta_path = session
|
||||||
|
.path()
|
||||||
|
.join("src/Workspace/TestModel/init.meta.json5");
|
||||||
|
poll_meta_has_ref_attr(
|
||||||
|
&meta_path,
|
||||||
|
"Rojo_Ref_PrimaryPart",
|
||||||
|
"Workspace/ModuleTarget.luau",
|
||||||
|
);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Fix 3 regression: same-batch add+ref for a Folder target (directory
|
||||||
|
/// middleware) should produce a path WITHOUT an extension.
|
||||||
|
#[test]
|
||||||
|
fn ref_same_batch_add_folder_target_no_extension() {
|
||||||
|
run_serve_test("ref_two_way_sync", |session, _| {
|
||||||
|
let (session_id, workspace_id, model_id, _, _, _) = ref_test_setup(&session);
|
||||||
|
|
||||||
|
let target_guid = Ref::new();
|
||||||
|
let added_inst = AddedInstance {
|
||||||
|
parent: Some(workspace_id),
|
||||||
|
name: "FolderTarget".to_string(),
|
||||||
|
class_name: "Folder".to_string(),
|
||||||
|
properties: HashMap::new(),
|
||||||
|
children: vec![],
|
||||||
|
};
|
||||||
|
let mut added_map = HashMap::new();
|
||||||
|
added_map.insert(target_guid, added_inst);
|
||||||
|
|
||||||
|
let mut update_props = UstrMap::default();
|
||||||
|
update_props.insert(ustr("PrimaryPart"), Some(Variant::Ref(target_guid)));
|
||||||
|
|
||||||
|
let write_request = WriteRequest {
|
||||||
|
session_id,
|
||||||
|
removed: vec![],
|
||||||
|
added: added_map,
|
||||||
|
updated: vec![InstanceUpdate {
|
||||||
|
id: model_id,
|
||||||
|
changed_name: None,
|
||||||
|
changed_class_name: None,
|
||||||
|
changed_properties: update_props,
|
||||||
|
changed_metadata: None,
|
||||||
|
}],
|
||||||
|
};
|
||||||
|
session.post_api_write(&write_request).unwrap();
|
||||||
|
thread::sleep(Duration::from_millis(500));
|
||||||
|
|
||||||
|
let meta_path = session
|
||||||
|
.path()
|
||||||
|
.join("src/Workspace/TestModel/init.meta.json5");
|
||||||
|
poll_meta_has_ref_attr(
|
||||||
|
&meta_path,
|
||||||
|
"Rojo_Ref_PrimaryPart",
|
||||||
|
"Workspace/FolderTarget",
|
||||||
|
);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue