diff --git a/.cursor/plans/luau-style_ref_paths_845083e1.plan.md b/.cursor/plans/luau-style_ref_paths_845083e1.plan.md new file mode 100644 index 00000000..3fc1a242 --- /dev/null +++ b/.cursor/plans/luau-style_ref_paths_845083e1.plan.md @@ -0,0 +1,1034 @@ +--- +name: Luau-style ref paths +overview: "Migrate Rojo_Ref_* attribute paths from bare absolute paths to Luau require-by-string style: @self/, ./, ../, and @game/. Prefer relative paths for maximum copy/paste portability. Update all syncback, forward sync, and two-way sync codepaths, plus comprehensive tests." +todos: + - id: core-compute + content: Add compute_relative_ref_path() to rojo_ref.rs + unit tests for all prefix selection scenarios + status: completed + - id: resolve-fn + content: Add resolve_ref_path() to tree.rs that handles @game/, @self/, ./, ../ prefixes and .. mid-path + status: completed + - id: forward-sync + content: Update patch_compute.rs and patch_apply.rs to pass source_ref when resolving Rojo_Ref_* paths + status: completed + - id: syncback-placeholders + content: Redesign placeholder system in ref_properties.rs to encode (source, target) pairs, compute relative paths in post-processing + status: completed + - id: syncback-final + content: Update fs_snapshot.rs fix_ref_paths and syncback/mod.rs post-processing to produce relative substitutions + status: completed + - id: two-way-write + content: Update api.rs syncback_updated_properties and added_paths to write relative ref paths + status: completed + - id: change-processor + content: Rewrite update_ref_paths_after_rename in change_processor.rs to resolve-then-recompute instead of string prefix replacement + status: completed + - id: ref-path-index + content: Change RefPathIndex.populate_from_dir to populate_from_tree, store resolved absolute paths, update serve_session.rs call site + status: completed + - id: update-existing-tests + content: Fix all existing test fixtures and assertions to use new prefix format + status: completed + - id: new-test-compute + content: Add comprehensive compute_relative_ref_path unit tests (all scenarios from plan) + status: completed + - id: new-test-resolve + content: Add comprehensive resolve_ref_path unit tests (all prefix types, .., edge cases) + status: completed + - id: new-test-roundtrip + content: Add syncback round-trip integration tests with relative refs (@self, ./, ../, @game) + status: completed + - id: new-test-twoway + content: Add two-way sync integration tests (ref writes, renames, dedup cleanup with relative paths) + status: completed + - id: new-test-edge + content: Add edge case tests (cross-service, ancestor refs, slugified names, dedup suffixes, same-batch add+ref) + status: completed + - id: cleanup + content: Remove dead escape/unescape functions, update documentation comments throughout + status: completed +isProject: false +--- + +# Luau-Style Ref Paths + +## Current State + +`Rojo_Ref_*` attributes store bare absolute paths from DataModel's children: + +```json5 +{ "Rojo_Ref_PrimaryPart": "Workspace/TestModel/Part1.model.json5" } +``` + +## Target State + +Use Luau require-by-string semantics with four prefix types: + +```json5 +// Descendant of source instance +{ "Rojo_Ref_PrimaryPart": "@self/Part1.model.json5" } + +// Sibling (child of parent) +{ "Rojo_Ref_Value": "./Sibling.model.json5" } + +// Navigate up then down (within same service) +{ "Rojo_Ref_Attachment0": "../Attachments/Att1.model.json5" } + +// Cross-service or target is ancestor of source +{ "Rojo_Ref_Value": "@game/ReplicatedStorage/Shared/Module.luau" } +``` + +## Prefix Selection Algorithm + +Given source and target absolute ref paths: + +1. **target == source** --> `@self` (self-reference, rare) +2. **target is descendant of source** --> `@self/` +3. **LCA is DataModel root** (cross-service, `common_prefix_len == 0`) --> `@game/` +4. **target is ancestor of source** (`remaining` is empty after LCA) --> `@game/` +5. **Otherwise** (same service, different branches): + - `ups == 1` (LCA is parent) --> `./` + - `ups >= 2` --> `"../" * (ups - 1)` + `` + +This maximizes portability: subtrees can be copied/moved within a service without breaking refs. + +**Worked examples:** + + +| Source | Target | LCA | Result | +| ---------------------- | ------------------------------- | ------------- | ------------------------------------- | +| `Workspace/A/Script` | `Workspace/A/Script/Child` | (descendant) | `@self/Child` | +| `Workspace/A/Script` | `Workspace/A/Part.model.json5` | `Workspace/A` | `./Part.model.json5` | +| `Workspace/A/Script` | `Workspace/B/Part.model.json5` | `Workspace` | `../B/Part.model.json5` | +| `Workspace/A/B/Script` | `Workspace/C/Part.model.json5` | `Workspace` | `../../C/Part.model.json5` | +| `Workspace/A/Script` | `ReplicatedStorage/Module.luau` | root | `@game/ReplicatedStorage/Module.luau` | +| `Workspace/A/B/Script` | `Workspace/A` | (ancestor) | `@game/Workspace/A` | + + +## Resolution Algorithm + +Per the [Roblox require-by-string docs](https://create.roblox.com/docs/reference/engine/globals/LuaGlobals#require): + +``` +resolve_ref_path(path, source_ref, tree): + @game/rest --> resolve segments from root (game) + @self --> return source_ref (script itself) + @self/rest --> resolve segments from source_ref (script) + ./rest --> resolve segments from parent(source_ref) (script.Parent) + ../rest --> resolve segments from parent(parent(source_ref)) (script.Parent.Parent) + bare_path --> resolve segments from root (backward compat fallback) + + During segment walk: + ".." --> go to parent of previous component + other --> find child by filesystem name (case-insensitive) +``` + +**How `../` chaining works** (from Roblox docs): + +- `../X` = script.Parent.Parent.X (2 ups total) +- `../../X` = script.Parent.Parent.Parent.X (3 ups -- `../` prefix gives 2, then `..` component gives 1 more) +- `../../../X` = 4 ups (prefix gives 2, two `..` components give 2 more) + +**Generation formula** (ups = levels from source to LCA): + +- ups=1: `"./" + remaining` +- ups>=2: `"../" * (ups-1) + remaining` + +Verification: ups=3 --> `"../" * 2` = `"../../"` + remaining. Parse: `../` prefix = 2 ups, one `..` component = 1 more = 3. Correct. + +Bare paths (no prefix) are treated as `@game/` for backward compatibility with old-format files. New writes never produce bare paths. + +--- + +## Phase 1: Core Path Functions + +**File: [src/rojo_ref.rs](src/rojo_ref.rs)** + +- Add `compute_relative_ref_path(source_abs: &str, target_abs: &str) -> String` -- pure string function implementing the prefix selection algorithm above. Both inputs are absolute ref paths (slash-separated filesystem names from root children). Output is the prefixed relative/absolute path. +- Keep `ref_target_path_from_tree()` and `ref_target_path()` for computing absolute paths (they become internal building blocks). +- The escape/unescape functions (`escape_ref_path_segment`, `split_ref_path`, etc.) become dead code since filesystem names can't contain `/`. Remove or deprecate them. + +## Phase 2: Path Resolution (Forward Sync) + +**File: [src/snapshot/tree.rs](src/snapshot/tree.rs)** + +- Add `resolve_ref_path(&self, path: &str, source_ref: Ref) -> Option` -- new public function. Parses prefix, sets starting Ref, then walks segments. Handles `..` as "go to parent" at any position. +- `get_instance_by_path()` remains as internal helper for absolute segment walking (used by `resolve_ref_path` for the `@game/` and bare-path cases). + +**File: [src/snapshot/patch_compute.rs](src/snapshot/patch_compute.rs)** + +- `compute_ref_properties()` currently calls `tree.get_instance_by_path(path)`. Change to call `tree.resolve_ref_path(path, source_ref)` where `source_ref` is the instance that owns the attributes. + +**File: [src/snapshot/patch_apply.rs](src/snapshot/patch_apply.rs)** + +- `finalize_patch_application()` resolves deferred refs in `path_refs_to_rewrite`. Currently uses `tree.get_instance_by_path()`. Change to `tree.resolve_ref_path(path, source_ref)` where source_ref comes from the `MultiMap` key. + +## Phase 3: CLI Syncback (Writing Relative Paths) + +**File: [src/syncback/ref_properties.rs](src/syncback/ref_properties.rs)** + +- **Placeholder redesign:** Placeholders must be unique per (source, target) pair since the same target gets different relative paths from different sources. Change from `__ROJO_REF_`__ to `__ROJO_REF__TO_`__. +- Change `placeholder_to_target: HashMap` to `placeholder_to_source_and_target: HashMap`. +- When `final_paths` IS available, call `compute_relative_ref_path(source_path, target_path)` directly instead of storing the absolute target path. +- `collect_all_paths()` is unchanged (still collects absolute paths for all instances). + +**File: [src/syncback/fs_snapshot.rs](src/syncback/fs_snapshot.rs)** + +- `fix_ref_paths()` substitution map entries are now `(placeholder, relative_path)`. The post-processing step in [src/syncback/mod.rs](src/syncback/mod.rs) must look up BOTH source and target in `ref_path_map`, call `compute_relative_ref_path()`, and produce the substitution. + +**File: [src/syncback/snapshot.rs](src/syncback/snapshot.rs)** + +- `record_ref_path()` is unchanged -- it still records absolute paths in `ref_path_map`. These are used as inputs to `compute_relative_ref_path`. + +## Phase 4: Two-Way Sync (Writing Relative Paths) + +**File: [src/web/api.rs](src/web/api.rs)** + +- `syncback_updated_properties()`: Currently computes target absolute path via `ref_target_path_from_tree()` and stores it directly. Change to also compute source absolute path, then call `compute_relative_ref_path(source_abs, target_abs)` for the attribute value. +- `added_paths` pre-computation: Same change -- compute relative paths from source to the newly-added target. + +**File: [src/change_processor.rs](src/change_processor.rs)** + +- `update_ref_paths_after_rename()`: Currently does string prefix replacement on absolute paths. With relative paths on disk, the approach changes: + 1. Find affected files via RefPathIndex (still indexed by absolute target path) + 2. For each file, parse JSON, resolve each `Rojo_Ref`_* value to absolute (using source instance's absolute path) + 3. If resolved absolute matches old prefix, compute new absolute, then recompute relative path from source + 4. Write updated relative path back +- This requires knowing each file's source instance absolute path. Add a helper that derives this from the file's filesystem path + tree metadata (instigating source lookup). + +## Phase 5: RefPathIndex Updates + +**File: [src/rojo_ref.rs*](src/rojo_ref.rs)* (RefPathIndex) + +The index stores **resolved absolute paths** as keys (not the on-disk relative strings). This preserves the efficient prefix-based lookup. + +- `populate_from_dir()` --> Change to `populate_from_tree(tree: &RojoTree)`. Walk instances in the tree, read `Rojo_Ref`_* attributes, resolve relative paths to absolute using the source instance's position, index the absolute path. Called after initial tree build in [src/serve_session.rs](src/serve_session.rs) line 177-181. +- `add()` / `remove()`: Callers pass resolved absolute paths (same as before). +- `find_by_prefix()` / `update_prefix()`: Unchanged (operate on absolute keys). +- `update_ref_paths_in_file()` in [src/syncback/meta.rs](src/syncback/meta.rs): Rewrite to parse + resolve + recompute instead of string prefix replacement. + +## Phase 6: Tests + +### Update Existing Tests + +All test fixtures and assertions with bare absolute ref paths need updating to use the new prefix format. + +**Key files:** + +- [rojo-test/serve-tests/ref_two_way_sync/](rojo-test/serve-tests/ref_two_way_sync/) -- fixture meta files +- [rojo-test/serve-tests/ref_forward_sync/](rojo-test/serve-tests/ref_forward_sync/) -- fixture meta files +- [tests/tests/two_way_sync.rs](tests/tests/two_way_sync.rs) -- ref assertions +- [tests/tests/serve.rs](tests/tests/serve.rs) -- ref assertions +- [src/rojo_ref.rs](src/rojo_ref.rs) -- unit tests +- [src/syncback/ref_properties.rs](src/syncback/ref_properties.rs) -- unit tests +- [src/syncback/fs_snapshot.rs](src/syncback/fs_snapshot.rs) -- fix_ref_paths tests +- [src/snapshot/tests/](src/snapshot/tests/) -- patch compute/apply tests + +--- + +### Test Specification: `compute_relative_ref_path(source_abs, target_abs)` + +Every test below shows the two absolute path inputs and the exact expected output string. + +#### `@self` -- Target is the source itself or a descendant + + +| # | Source | Target | Expected | Why | +| --- | ----------------- | -------------------------------------------- | ---------------------------------- | ---------------------------------------- | +| 1 | `Workspace/Model` | `Workspace/Model` | `@self` | Self-reference | +| 2 | `Workspace/Model` | `Workspace/Model/Handle.model.json5` | `@self/Handle.model.json5` | Direct child | +| 3 | `Workspace/Model` | `Workspace/Model/SubFolder/Part.model.json5` | `@self/SubFolder/Part.model.json5` | Nested descendant | +| 4 | `Workspace/Model` | `Workspace/Model/A/B/C/D.luau` | `@self/A/B/C/D.luau` | Deeply nested descendant (4 levels) | +| 5 | `Workspace/Tool` | `Workspace/Tool/Handle.model.json5` | `@self/Handle.model.json5` | Tool with Handle (common Roblox pattern) | +| 6 | `Workspace/Model` | `Workspace/Model/Init` | `@self/Init` | Child that is a directory (init-style) | +| 7 | `Workspace/Gun` | `Workspace/Gun/Sounds/Fire.model.json5` | `@self/Sounds/Fire.model.json5` | Tool referencing nested sound | +| 8 | `Workspace/Model` | `Workspace/Model/Hey_Bro.server.luau` | `@self/Hey_Bro.server.luau` | Slugified child name | +| 9 | `Workspace/Model` | `Workspace/Model/Data~2` | `@self/Data~2` | Dedup'd child | + + +#### `./` -- Target is a sibling (shares same parent, 1 level up) + + +| # | Source | Target | Expected | Why | +| --- | ------------------------------------------ | -------------------------------------- | ----------------------- | -------------------------------------- | +| 10 | `Workspace/Folder/ScriptA.server.luau` | `Workspace/Folder/ScriptB.server.luau` | `./ScriptB.server.luau` | Sibling script | +| 11 | `Workspace/Folder/ScriptA.server.luau` | `Workspace/Folder/Config.luau` | `./Config.luau` | Sibling module | +| 12 | `Workspace/Folder/ScriptA.server.luau` | `Workspace/Folder/SubFolder` | `./SubFolder` | Sibling directory | +| 13 | `Workspace/Folder/ScriptA.server.luau` | `Workspace/Folder/SubFolder/Deep.luau` | `./SubFolder/Deep.luau` | Sibling then descend | +| 14 | `Workspace/Folder/ObjectValue.model.json5` | `Workspace/Folder/Target.model.json5` | `./Target.model.json5` | ObjectValue.Value pointing to sibling | +| 15 | `Workspace/Beams/Beam.model.json5` | `Workspace/Beams/Att1.model.json5` | `./Att1.model.json5` | Beam.Attachment0 to sibling attachment | +| 16 | `Workspace/Beams/Beam.model.json5` | `Workspace/Beams/Att2.model.json5` | `./Att2.model.json5` | Beam.Attachment1 to another sibling | +| 17 | `Workspace/Folder/A.luau` | `Workspace/Folder/Hey_Bro.server.luau` | `./Hey_Bro.server.luau` | Sibling with slugified name | +| 18 | `Workspace/Folder/A.luau` | `Workspace/Folder/Data~2` | `./Data~2` | Sibling with dedup suffix | +| 19 | `Workspace/Folder/A.luau` | `Workspace/Folder/Data~3.model.json5` | `./Data~3.model.json5` | Sibling model with dedup suffix | + + +#### `../` -- Navigate up 2+ levels then down (within same service) + + +| # | Source | Target | Expected | Why | +| --- | ----------------------------------------------- | ---------------------------------------------- | -------------------------------- | ------------------------------------------- | +| 20 | `Workspace/A/Script.server.luau` | `Workspace/B/Part.model.json5` | `../B/Part.model.json5` | Parent's sibling's child (2 ups) | +| 21 | `Workspace/A/Script.server.luau` | `Workspace/B` | `../B` | Parent's sibling directory (2 ups) | +| 22 | `Workspace/A/B/Script.server.luau` | `Workspace/C/Part.model.json5` | `../../C/Part.model.json5` | Grandparent's sibling's child (3 ups) | +| 23 | `Workspace/A/B/C/Script.luau` | `Workspace/D/E/Part.model.json5` | `../../../D/E/Part.model.json5` | 4 ups within same service | +| 24 | `Workspace/A/B/C/D/Script.luau` | `Workspace/E/Part.model.json5` | `../../../../E/Part.model.json5` | 5 ups within same service | +| 25 | `Workspace/Models/Car/Body.model.json5` | `Workspace/Models/Truck/Body.model.json5` | `../Truck/Body.model.json5` | Peer model in sibling folder | +| 26 | `Workspace/A/Deep/Script.luau` | `Workspace/A/Other.luau` | `../Other.luau` | Up from deep, target is cousin (2 ups) | +| 27 | `Workspace/A/B/Beam.model.json5` | `Workspace/A/C/Att.model.json5` | `../C/Att.model.json5` | Beam referencing attachment in uncle folder | +| 28 | `Workspace/Systems/Combat/Hitbox.server.luau` | `Workspace/Systems/Audio/HitSound.model.json5` | `../Audio/HitSound.model.json5` | Cross-system ref within same parent | +| 29 | `Workspace/Map/Zone1/Spawns/Spawn1.model.json5` | `Workspace/Map/Zone1/Props/Tree.model.json5` | `../Props/Tree.model.json5` | Same zone, different category | +| 30 | `Workspace/Map/Zone1/Spawns/Spawn1.model.json5` | `Workspace/Map/Zone2/Flag.model.json5` | `../../Zone2/Flag.model.json5` | Cross-zone ref | + + +#### `@game/` -- Cross-service or target is ancestor + + +| # | Source | Target | Expected | Why | +| --- | ----------------------------------------- | -------------------------------------------- | -------------------------------------------------- | ------------------------------------- | +| 31 | `Workspace/Script.server.luau` | `ReplicatedStorage/Module.luau` | `@game/ReplicatedStorage/Module.luau` | Classic cross-service require pattern | +| 32 | `Workspace/Script.server.luau` | `ServerStorage/Data.luau` | `@game/ServerStorage/Data.luau` | Server script to ServerStorage | +| 33 | `ServerScriptService/Main.server.luau` | `ReplicatedStorage/Shared/Utils.luau` | `@game/ReplicatedStorage/Shared/Utils.luau` | SSS to RS (common pattern) | +| 34 | `StarterGui/ScreenGui/Button.client.luau` | `ReplicatedStorage/UI/Theme.luau` | `@game/ReplicatedStorage/UI/Theme.luau` | Client UI to shared module | +| 35 | `Workspace/A/B/C/Script.luau` | `Lighting` | `@game/Lighting` | Ref to a service itself | +| 36 | `Workspace/A/B/C/Script.luau` | `Workspace` | `@game/Workspace` | Ref to own service (ancestor) | +| 37 | `Workspace/A/B/Script.luau` | `Workspace/A` | `@game/Workspace/A` | Ref to own grandparent (ancestor) | +| 38 | `Workspace/A/B/C/Script.luau` | `Workspace/A/B` | `@game/Workspace/A/B` | Ref to own great-grandparent | +| 39 | `Workspace/Model` | `ReplicatedStorage/Assets/Model.model.json5` | `@game/ReplicatedStorage/Assets/Model.model.json5` | Cross-service model ref | +| 40 | `Workspace/Beam.model.json5` | `Lighting/Atmosphere.model.json5` | `@game/Lighting/Atmosphere.model.json5` | Workspace to Lighting | +| 41 | `ReplicatedStorage/A.luau` | `ReplicatedStorage` | `@game/ReplicatedStorage` | Ref to own service (ancestor) | +| 42 | `Workspace/Script.luau` | `SoundService/BGM.model.json5` | `@game/SoundService/BGM.model.json5` | To SoundService | + + +#### Special names: slugified, dedup'd, init-style + + +| # | Source | Target | Expected | Why | +| --- | ------------------------- | -------------------------------------- | ----------------------- | ----------------------------------------------- | +| 43 | `Workspace/Folder/A.luau` | `Workspace/Folder/Hey_Bro.server.luau` | `./Hey_Bro.server.luau` | Sibling with slug (real name "Hey/Bro") | +| 44 | `Workspace/Folder/A.luau` | `Workspace/Other/CON_.luau` | `../Other/CON_.luau` | Windows reserved name (slugified) | +| 45 | `Workspace/A.luau` | `Workspace/Data~2` | `./Data~2` | Dedup'd sibling (Folder) | +| 46 | `Workspace/A.luau` | `Workspace/Data~3.model.json5` | `./Data~3.model.json5` | Dedup'd model sibling | +| 47 | `Workspace/A.luau` | `Workspace/Data~2/Child.luau` | `./Data~2/Child.luau` | Into dedup'd folder | +| 48 | `Workspace/Model` | `Workspace/Model/Scripts` | `@self/Scripts` | Child is init-style directory | +| 49 | `Workspace/Scripts` | `Workspace/Scripts/Foo` | `@self/Foo` | Source is init-style, child is too | +| 50 | `Workspace/MyFolder`_ | `Workspace/MyFolder_/Child.luau` | `@self/Child.luau` | Source name has trailing underscore (from slug) | + + +#### Tricky edge cases + + +| # | Source | Target | Expected | Why | +| --- | ----------------------------------- | ----------------------------------- | ------------------------------------- | ------------------------------------------------------------ | +| 51 | `Workspace` | `Workspace/Model.model.json5` | `@self/Model.model.json5` | Service referencing own child | +| 52 | `Workspace` | `ReplicatedStorage/Module.luau` | `@game/ReplicatedStorage/Module.luau` | Service to service (cross-service) | +| 53 | `Workspace` | `Workspace` | `@self` | Service self-ref | +| 54 | `Workspace` | `Lighting` | `@game/Lighting` | Service to different service | +| 55 | `Workspace/A` | `Workspace/AB` | `./AB` | Prefix ambiguity: "A" is NOT prefix of "AB" in segment terms | +| 56 | `Workspace/Foo.server.luau` | `Workspace/Foo.luau` | `./Foo.luau` | Same stem different extension = different instances | +| 57 | `Workspace/A/B/C/D/E/F/Script.luau` | `Workspace/A/B/C/D/E/G/Target.luau` | `../G/Target.luau` | Deep nesting, close cousins | +| 58 | `Workspace/A/B/C/D/E/F/Script.luau` | `Workspace/X/Target.luau` | `../../../../../X/Target.luau` | Deep nesting, far apart but same service | + + +--- + +### Test Specification: `resolve_ref_path(path, source_ref, tree)` + +Build a RojoTree for each test. The tree structure is shown in comments. Each test verifies that the path resolves to the correct Ref (or None). + +#### Prefix resolution + +Tree for tests 59-78 (instance hierarchy with filesystem names in parens): + +``` +DataModel (root) + Workspace + ModelA (Model, fs: "ModelA") + Script (Script server, fs: "Script.server.luau") + Handle (Part, fs: "Handle.model.json5") + SubFolder (Folder, fs: "SubFolder") + Deep (Part, fs: "Deep.model.json5") + ModelB (Folder, fs: "ModelB") + Part (Part, fs: "Part.model.json5") + ReplicatedStorage + Module (ModuleScript, fs: "Module.luau") + Shared (Folder, fs: "Shared") + Utils (ModuleScript, fs: "Utils.luau") + Lighting +``` + +Key relationships: Script and Handle are **siblings** under ModelA. ModelA **has children** (Script, Handle, SubFolder). ModelA and ModelB are **siblings** under Workspace. + + +| # | Path | Source | Expected | Why | +| --- | ------------------------------------------- | ------ | -------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 59 | `@self/Handle.model.json5` | ModelA | Handle | `@self/` starts at ModelA, child Handle | +| 60 | `@self/SubFolder/Deep.model.json5` | ModelA | Deep | `@self/` nested descent from ModelA | +| 61 | `@self` | Script | Script | Self-reference (no path after prefix) | +| 62 | `./Handle.model.json5` | Script | Handle | `./` starts at Script.Parent = ModelA, child Handle | +| 63 | `./SubFolder/Deep.model.json5` | Script | Deep | `./` starts at ModelA, descend through SubFolder | +| 64 | `./Script.server.luau` | Handle | Script | `./` from Handle.Parent = ModelA, child Script | +| 65 | `../ModelB/Part.model.json5` | Script | Part | `../` starts at Script.Parent.Parent = Workspace, then ModelB/Part | +| 66 | `../../ReplicatedStorage/Module.luau` | Script | Module | `../` starts at Workspace; remaining `../ReplicatedStorage/Module.luau` goes `..` up to DataModel root, then descends. Valid path (just not one we'd generate -- we'd use `@game/` instead) | +| 67 | `@game/ReplicatedStorage/Module.luau` | Script | Module | `@game/` absolute from root | +| 68 | `@game/Workspace/ModelA/Handle.model.json5` | Script | Handle | `@game/` works even when target is nearby | +| 69 | `@game/Lighting` | Script | Lighting | `@game/` to a service | +| 70 | `Workspace/ModelA/Handle.model.json5` | Script | Handle | Bare path (no prefix) = backward compat, resolves like `@game/` | +| 71 | `@game/Workspace/ModelA/NonExistent` | Script | None | Target doesn't exist | +| 72 | `./NonExistent.luau` | Script | None | Sibling doesn't exist | + + +#### `..` mid-path + + +| # | Path | Source | Expected | Why | +| --- | ------------------------------------------- | ------ | -------- | ------------------------------------------------------------------------------------ | +| 73 | `@self/SubFolder/../Handle.model.json5` | ModelA | Handle | Start at ModelA, into SubFolder, `..` back to ModelA, child Handle | +| 74 | `./SubFolder/../Script.server.luau` | Handle | Script | `./` at ModelA, into SubFolder, `..` back to ModelA, child Script | +| 75 | `./SubFolder/../../ModelB/Part.model.json5` | Script | Part | `./` at ModelA, into SubFolder, `..` to ModelA, `..` to Workspace, child ModelB/Part | + + +#### Case-insensitive matching + + +| # | Path | Source | Expected | Why | +| --- | ------------------------------------------- | ------ | -------- | ---------------------------------------- | +| 76 | `@self/handle.model.json5` | ModelA | Handle | Lowercase "handle" matches "Handle" | +| 77 | `./SCRIPT.SERVER.LUAU` | Handle | Script | All-caps matches filesystem name | +| 78 | `@game/workspace/modela/Handle.model.json5` | Script | Handle | Lowercase service and folder names match | + + +#### Filesystem name vs instance name fallback + +Tree with metadata: + +``` +DataModel + Workspace + MyFolder (fs_name: "MyFolder", instance_name: "MyFolder") + Slugged (fs_name: "Hey_Bro.server.luau", instance_name: "Hey/Bro") + Normal.luau (fs_name: "Normal.luau", instance_name: "Normal") +``` + + +| # | Path | Source | Expected | Why | +| --- | ---------------------------------------------- | ------ | -------- | -------------------------- | +| 79 | `./Hey_Bro.server.luau` | Normal | Slugged | Matches by filesystem name | +| 80 | `@game/Workspace/MyFolder/Hey_Bro.server.luau` | Normal | Slugged | Absolute with fs name | + + +--- + +### Test Specification: Syncback Round-Trip + +Each test: build a WeakDom with specific instances and Ref properties, run syncback, verify the on-disk `Rojo_Ref_*` attribute values, then rebuild from disk and verify the Ref properties resolve to the same targets. + +#### RT-1: Model with PrimaryPart (`@self/`) + +``` +Workspace + TestModel (Model) + PrimaryPart → Handle + Handle (Part) + Other (Part) +``` + +**Expected on disk** (`Workspace/TestModel/init.meta.json5`): + +```json5 +{ "className": "Model", "attributes": { "Rojo_Ref_PrimaryPart": "@self/Handle.model.json5" } } +``` + +Rebuild: `TestModel.PrimaryPart` == Handle. Round-trip identical. + +#### RT-2: Cross-service ObjectValue (`@game/`) + +``` +Workspace + OV (ObjectValue) + Value → Target +ReplicatedStorage + Target (ModuleScript, source="return {}") +``` + +**Expected on disk** (`Workspace/OV.model.json5`): + +```json5 +{ "className": "ObjectValue", "attributes": { "Rojo_Ref_Value": "@game/ReplicatedStorage/Target.luau" } } +``` + +#### RT-3: Beam with attachments in same parent (`./`) + +``` +Workspace + Beams (Folder) + Beam (Beam) + Attachment0 → Att1 + Attachment1 → Att2 + Att1 (Attachment) + Att2 (Attachment) +``` + +**Expected on disk** (`Workspace/Beams/Beam.model.json5`): + +```json5 +{ + "className": "Beam", + "attributes": { + "Rojo_Ref_Attachment0": "./Att1.model.json5", + "Rojo_Ref_Attachment1": "./Att2.model.json5" + } +} +``` + +#### RT-4: Beam with attachments in sibling subfolder (`./`) + +``` +Workspace + Beams (Folder) + Beam (Beam) + Attachment0 → Att1 + Attachments (Folder) + Att1 (Attachment) +``` + +Beam and Attachments are siblings under Beams. Source = `Workspace/Beams/Beam.model.json5`. Target = `Workspace/Beams/Attachments/Att1.model.json5`. LCA = `Workspace/Beams`. ups = 1 --> `./`. + +**Expected on disk** (`Workspace/Beams/Beam.model.json5`): + +```json5 +{ + "className": "Beam", + "attributes": { "Rojo_Ref_Attachment0": "./Attachments/Att1.model.json5" } +} +``` + +#### RT-5: Beam with attachments across branches (`../`) + +``` +Workspace + GroupA (Folder) + Beam (Beam) + Attachment0 → Att1 + GroupB (Folder) + Att1 (Attachment) +``` + +**Expected on disk** (`Workspace/GroupA/Beam.model.json5`): + +```json5 +{ "className": "Beam", "attributes": { "Rojo_Ref_Attachment0": "../GroupB/Att1.model.json5" } } +``` + +Source = `Workspace/GroupA/Beam.model.json5`. Target = `Workspace/GroupB/Att1.model.json5`. LCA = `Workspace`. ups = 2. `../GroupB/Att1.model.json5`. + +#### RT-6: Mixed prefix types across multiple meta files + +``` +Workspace + Container (Folder) + Model (Model) + Handle (Part) <-- child of Model + RefToSibling (ObjectValue, Value → Sibling) <-- child of Model + Sibling (Part) <-- child of Container (sibling of Model) +ReplicatedStorage + SharedModule (ModuleScript) +``` + +Two refs produce different prefix types: + +1. **Model.PrimaryPart → Handle**: Source = Model (`Workspace/Container/Model`). Handle is a descendant of Model --> `@self/Handle.model.json5`. Written in `Model/init.meta.json5`. +2. **RefToSibling.Value → Sibling**: Source = RefToSibling (`Workspace/Container/Model/RefToSibling.model.json5`). Target = Sibling (`Workspace/Container/Sibling.model.json5`). LCA = Container. ups = 2 --> `../Sibling.model.json5`. Written in `RefToSibling.model.json5`. + +**Expected** (`Workspace/Container/Model/init.meta.json5`): + +```json5 +{ "className": "Model", "attributes": { "Rojo_Ref_PrimaryPart": "@self/Handle.model.json5" } } +``` + +**Expected** (`Workspace/Container/Model/RefToSibling.model.json5`): + +```json5 +{ "className": "ObjectValue", "attributes": { "Rojo_Ref_Value": "../Sibling.model.json5" } } +``` + +Verifies two different ref prefix types coexist in the same directory tree. + +#### RT-7: Refs through dedup'd siblings + +``` +Workspace + Container (Folder) + Data (Folder, child1) + Data (Folder, child2) --> gets dedup'd to Data~2 + Pointer (ObjectValue) + Value → Data(child2) +``` + +**Expected** (`Workspace/Container/Pointer.model.json5`): + +```json5 +{ "className": "ObjectValue", "attributes": { "Rojo_Ref_Value": "./Data~2" } } +``` + +#### RT-8: Ref to init-style script + +``` +Workspace + Folder + Pointer (ObjectValue, Value → BigScript) + BigScript (ModuleScript with children) + Helper (ModuleScript) +``` + +BigScript has children, so it becomes a directory: `Folder/BigScript/init.luau`. Filesystem name = `BigScript`. + +**Expected** (`Workspace/Folder/Pointer.model.json5`): + +```json5 +{ "className": "ObjectValue", "attributes": { "Rojo_Ref_Value": "./BigScript" } } +``` + +#### RT-9: Ref to instance with slugified name + +``` +Workspace + Folder + Pointer (ObjectValue, Value → Target) + Target (Script, name="Hey:There", server) +``` + +"Hey:There" slugifies to "Hey_There". File = `Hey_There.server.luau`. + +**Expected** (`Workspace/Folder/Pointer.model.json5`): + +```json5 +{ "className": "ObjectValue", "attributes": { "Rojo_Ref_Value": "./Hey_There.server.luau" } } +``` + +#### RT-10: Deep nesting within same service + +``` +Workspace + A (Folder) + B (Folder) + C (Folder) + D (Folder) + Source (ObjectValue, Value → Target) + X (Folder) + Target (Part) +``` + +Source = `Workspace/A/B/C/D/Source.model.json5`. Target = `Workspace/A/X/Target.model.json5`. LCA = `Workspace/A`. ups = 4. --> `../../../X/Target.model.json5`. + +**Expected**: + +```json5 +{ "className": "ObjectValue", "attributes": { "Rojo_Ref_Value": "../../../X/Target.model.json5" } } +``` + +#### RT-11: Multiple instances referencing same target from different depths + +``` +Workspace + Target (Part) + Shallow (ObjectValue, Value → Target) + Deep (Folder) + Mid (Folder) + DeepOV (ObjectValue, Value → Target) +``` + +- Shallow: Source = `Workspace/Shallow.model.json5`, Target = `Workspace/Target.model.json5`. ups = 1. --> `./Target.model.json5` +- DeepOV: Source = `Workspace/Deep/Mid/DeepOV.model.json5`, Target = `Workspace/Target.model.json5`. LCA = `Workspace`. ups = 3. --> `../../Target.model.json5` + +Verifies same target gets different paths from different sources. + +#### RT-12: Duplicate-named attachments (dedup + placeholder system) + +``` +Workspace + Beams (Folder) + Att (Attachment, child1) + Att (Attachment, child2) --> Att~2 + Att (Attachment, child3) --> Att~3 + Beam (Beam) + Attachment0 → Att(child2) + Attachment1 → Att(child3) +``` + +**Expected** (`Workspace/Beams/Beam.model.json5`): + +```json5 +{ + "className": "Beam", + "attributes": { + "Rojo_Ref_Attachment0": "./Att~2.model.json5", + "Rojo_Ref_Attachment1": "./Att~3.model.json5" + } +} +``` + +Verifies source-aware placeholders produce correct per-source relative paths even with dedup. + +--- + +### Test Specification: Two-Way Sync + +#### TW-1: Ref property change writes relative path + +Setup: Serve session with tree: + +``` +Workspace + Model (Model) + Handle (Part) +``` + +Plugin sends: `Model.PrimaryPart = Handle` (Ref property change). + +**Verify** `init.meta.json5` on disk contains: + +```json5 +{ "attributes": { "Rojo_Ref_PrimaryPart": "@self/Handle.model.json5" } } +``` + +#### TW-2: Cross-service ref writes `@game/` + +Setup: + +``` +Workspace + OV (ObjectValue) +ReplicatedStorage + Target (ModuleScript) +``` + +Plugin sends: `OV.Value = Target`. + +**Verify** `OV.model.json5` contains: + +```json5 +{ "attributes": { "Rojo_Ref_Value": "@game/ReplicatedStorage/Target.luau" } } +``` + +#### TW-3: Nil ref removes attribute + +Setup: Model already has `Rojo_Ref_PrimaryPart: "@self/Handle.model.json5"` on disk. + +Plugin sends: `Model.PrimaryPart = nil`. + +**Verify**: `Rojo_Ref_PrimaryPart` is removed from the meta file. + +#### TW-4: Same-batch add + ref + +Plugin sends in one write request: + +- Add: new Part "NewPart" under Model +- Update: `Model.PrimaryPart = NewPart` + +**Verify**: meta file contains `Rojo_Ref_PrimaryPart` with relative path to the newly added instance (should be `@self/NewPart.model.json5`). + +#### TW-5: Rename target updates refs + +Setup: + +``` +Workspace + Folder + Pointer (ObjectValue, Value → OldName) + OldName (Part) +``` + +On disk: `Pointer.model.json5` has `"Rojo_Ref_Value": "./OldName.model.json5"`. + +Plugin renames `OldName` to `NewName`. + +**Verify**: `Pointer.model.json5` updated to `"Rojo_Ref_Value": "./NewName.model.json5"`. + +#### TW-6: Rename source does NOT change its own outgoing refs + +Setup: + +``` +Workspace + Folder + SourceOV (ObjectValue, Value → Target) + Target (Part) +``` + +On disk: `SourceOV.model.json5` has `"Rojo_Ref_Value": "./Target.model.json5"`. + +Plugin renames `SourceOV` to `RenamedOV`. + +**Verify**: `RenamedOV.model.json5` still has `"Rojo_Ref_Value": "./Target.model.json5"` (unchanged -- the relative relationship didn't change). + +#### TW-7: Dedup cleanup rename updates refs + +Setup: + +``` +Workspace + Folder + Data (Folder) + Data~2 (Folder) + Pointer (ObjectValue, Value → Data~2) +``` + +On disk: `Pointer.model.json5` has `"Rojo_Ref_Value": "./Data~2"`. + +Delete `Data` (the base). Dedup cleanup promotes `Data~2` --> `Data`. + +**Verify**: `Pointer.model.json5` updated to `"Rojo_Ref_Value": "./Data"`. + +#### TW-8: Base-name promotion with cross-branch ref + +Setup: + +``` +Workspace + GroupA + Pointer (ObjectValue, Value → Target) + GroupB + Target (Part) + Target~2 (Part) +``` + +On disk: `Pointer.model.json5` has `"Rojo_Ref_Value": "../GroupB/Target~2"`. + +Delete `Target` (base). Cleanup: `Target~2` --> `Target`. + +**Verify**: `Pointer.model.json5` updated to `"Rojo_Ref_Value": "../GroupB/Target"`. + +#### TW-9: Multiple referencing files with different relative paths + +Setup: + +``` +Workspace + Target (Part) + ShallowOV (ObjectValue, Value → Target) + Deep + DeepOV (ObjectValue, Value → Target) +``` + +Rename `Target` to `Renamed`. + +**Verify**: + +- `ShallowOV.model.json5`: `"Rojo_Ref_Value": "./Renamed.model.json5"` (was `./Target.model.json5`) +- `Deep/DeepOV.model.json5`: `"Rojo_Ref_Value":` ../Renamed.model.json5"`(was`../Target.model.json5`) + +Both files updated with correct relative paths from their respective positions. + +#### TW-10: Rename intermediate ancestor + +Setup: + +``` +Workspace + Parent + Child + Target (Part) + Pointer (ObjectValue, Value → Target) +``` + +On disk: `Pointer.model.json5` has `"Rojo_Ref_Value": "./Parent/Child/Target.model.json5"`. + +Rename `Parent` to `RenamedParent`. + +**Verify**: `Pointer.model.json5` updated to `"Rojo_Ref_Value": "./RenamedParent/Child/Target.model.json5"`. + +--- + +### Test Specification: Placeholder System (Syncback) + +#### PH-1: Source-aware placeholders are unique + +Build DOM with two ObjectValues pointing to the same target: + +``` +Workspace + Target (Part) + OV_A (ObjectValue, Value → Target) + OV_B (ObjectValue, Value → Target) +``` + +Call `collect_referents` without `final_paths`. + +**Verify**: + +- OV_A gets placeholder `__ROJO_REF__TO_`__ +- OV_B gets placeholder `__ROJO_REF__TO_`__ +- Placeholders are different strings +- `placeholder_to_source_and_target` maps each to `(source_ref, target_ref)` + +#### PH-2: Post-processing produces different relative paths for same target + +After walk, `ref_path_map` contains: + +- OV_A --> `Workspace/OV_A.model.json5` +- OV_B --> `Workspace/Deep/OV_B.model.json5` +- Target --> `Workspace/Target.model.json5` + +**Verify substitutions**: + +- OV_A's placeholder --> `./Target.model.json5` (sibling) +- OV_B's placeholder --> `../Target.model.json5` (2 ups) + +#### PH-3: Duplicate-named targets with source-aware placeholders + +``` +Workspace + Beams + Att (Attachment, child1) + Att (Attachment, child2) + Beam (Beam, Attachment0 → child1, Attachment1 → child2) +``` + +Both attachments are named "Att" so they get different Refs but tentatively the same path. Placeholders encode both source (Beam) and target (child1, child2) so they're unique. + +After walk with dedup: child1 = `Att.model.json5`, child2 = `Att~2.model.json5`. + +**Verify**: + +- Attachment0: `./Att.model.json5` +- Attachment1: `./Att~2.model.json5` +- No chaining corruption + +#### PH-4: With `final_paths` available, computes relative directly + +Call `collect_referents` WITH `final_paths`: + +``` +OV_A source path: "Workspace/OV_A.model.json5" +Target final path: "Workspace/Target.model.json5" +``` + +**Verify**: No placeholders generated. Path stored directly as `./Target.model.json5`. + +--- + +### Test Specification: RefPathIndex with Relative Paths + +#### RI-1: populate_from_tree indexes resolved absolute paths + +Tree has: + +``` +Workspace/Model/init.meta.json5 with Rojo_Ref_PrimaryPart: "@self/Handle.model.json5" +``` + +Source instance absolute path: `Workspace/Model`. Resolve `@self/Handle.model.json5` from Model = `Workspace/Model/Handle.model.json5`. + +**Verify**: Index contains key `Workspace/Model/Handle.model.json5` mapping to the meta file. + +#### RI-2: find_by_prefix still works with absolute keys + +After RI-1, call `find_by_prefix("Workspace/Model/Handle.model.json5")`. + +**Verify**: Returns the meta file path. + +#### RI-3: Relative refs from different files pointing to same target + +Two files: + +- `Workspace/A/Pointer.model.json5` with `"Rojo_Ref_Value": "./Target.model.json5"` (resolves to `Workspace/A/Target.model.json5`) +- `Workspace/B/Pointer.model.json5` with `"Rojo_Ref_Value": "../A/Target.model.json5"` (resolves to `Workspace/A/Target.model.json5`) + +**Verify**: Both files indexed under key `Workspace/A/Target.model.json5`. + +#### RI-4: Rename updates files with recomputed relative paths + +After RI-3, rename `Workspace/A/Target.model.json5` to `Workspace/A/Renamed.model.json5`. + +**Verify**: + +- `Workspace/A/Pointer.model.json5` updated to `"Rojo_Ref_Value": "./Renamed.model.json5"` +- `Workspace/B/Pointer.model.json5` updated to `"Rojo_Ref_Value": "../A/Renamed.model.json5"` +- Index key updated from `Workspace/A/Target.model.json5` to `Workspace/A/Renamed.model.json5` + +--- + +### Test Specification: `fix_ref_paths` (fs_snapshot.rs) + +#### FP-1: Source-aware substitution with different relative paths + +Snapshot has two meta files: + +``` +/test/ShallowOV.model.json5: + { "attributes": { "Rojo_Ref_Value": "__ROJO_REF_shallow_TO_target__" } } + +/test/Deep/DeepOV.model.json5: + { "attributes": { "Rojo_Ref_Value": "__ROJO_REF_deep_TO_target__" } } +``` + +Substitutions: + +- `__ROJO_REF_shallow_TO_target__` --> `./Target.model.json5` +- `__ROJO_REF_deep_TO_target__` --> `../Target.model.json5` + +**Verify**: Each file gets its own correct relative path. + +#### FP-2: Non-ref lines are never touched + +Meta file has a `properties.Description` containing "Rojo_Ref_" as a string value. + +**Verify**: Only lines with actual `Rojo_Ref`_ attribute keys are substituted. + +#### FP-3: Non-meta files are never touched + +A `.luau` file happens to contain a placeholder string in a comment. + +**Verify**: File is not modified. + +#### FP-4: Multiple refs in same file with mixed prefixes + +```json5 +{ + "attributes": { + "Rojo_Ref_PrimaryPart": "__ROJO_REF_model_TO_handle__", + "Rojo_Ref_Value": "__ROJO_REF_model_TO_target__" + } +} +``` + +Substitutions produce `@self/Handle.model.json5` and `@game/ReplicatedStorage/Target.luau`. + +**Verify**: Both substituted correctly in same file. + +--- + +### Test Specification: Backward Compatibility + +#### BC-1: Old bare paths resolve correctly + +Meta file on disk: `{ "attributes": { "Rojo_Ref_PrimaryPart": "Workspace/Model/Handle.model.json5" } }`. + +**Verify**: Forward sync resolves this to the correct Handle instance (bare path treated as `@game/`). + +#### BC-2: Old bare paths still work during transition + +Build from a project that has old-format `Rojo_Ref_`* values. Refs should resolve correctly. No errors or warnings that break the build. + +#### BC-3: Syncback overwrites old format with new + +Project has old bare paths on disk. Run syncback. **Verify** all `Rojo_Ref_`* values are rewritten with the correct prefix (`@self/`, `./`, `../`, or `@game/`). + +--- + +### Test Specification: Format Transition (syncback_format_transitions) + +#### FT-1: Old absolute --> new relative on re-syncback + +Project has `"Rojo_Ref_PrimaryPart": "Workspace/Model/Handle.model.json5"` (old format). + +Run syncback with same rbxl. + +**Verify**: File updated to `"Rojo_Ref_PrimaryPart": "@self/Handle.model.json5"`. + +#### FT-2: Mixed old and new formats in same project + +Some files have old bare paths, others have already been updated to new format. Run syncback. + +**Verify**: ALL files now use new format. No old bare paths remain. \ No newline at end of file diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_change_target_patch.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_change_target_patch.snap index 05c77a9b..6ce2b91d 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_change_target_patch.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_change_target_patch.snap @@ -15,7 +15,7 @@ body: Attributes: "!Attributes": Rojo_Ref_PrimaryPart: - "!String": Workspace/MyModel/OtherPart + "!String": "@self/OtherPart" PrimaryPart: "!Ref": id-4 id: id-3 diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_multiple_attrs_patch.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_multiple_attrs_patch.snap index a899dd0c..b369de5c 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_multiple_attrs_patch.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_multiple_attrs_patch.snap @@ -15,9 +15,9 @@ body: Attributes: "!Attributes": Rojo_Ref_CustomRef: - "!String": Workspace/MyModel/OtherPart + "!String": "@self/OtherPart" Rojo_Ref_PrimaryPart: - "!String": Workspace/MyModel/Target + "!String": "@self/Target" CustomRef: "!Ref": id-4 id: id-3 diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_new_file_patch.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_new_file_patch.snap index 92a5b3b5..c38c7864 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_new_file_patch.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_new_file_patch.snap @@ -28,7 +28,7 @@ body: Attributes: "!Attributes": Rojo_Ref_PrimaryPart: - "!String": Workspace/MyModel/Target + "!String": "@self/Target" id: id-3 packetType: messages sessionId: id-1 diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_nonexistent_patch.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_nonexistent_patch.snap index a9eee0e2..2cafeb60 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_nonexistent_patch.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_nonexistent_patch.snap @@ -15,7 +15,7 @@ body: Attributes: "!Attributes": Rojo_Ref_PrimaryPart: - "!String": Workspace/NonExistent/Part + "!String": "./NonExistent/Part" PrimaryPart: ~ id: id-3 packetType: messages diff --git a/rojo-test/serve-tests/ref_forward_sync/src/Workspace/MyModel/init.meta.json5 b/rojo-test/serve-tests/ref_forward_sync/src/Workspace/MyModel/init.meta.json5 index d83679ff..b6425515 100644 --- a/rojo-test/serve-tests/ref_forward_sync/src/Workspace/MyModel/init.meta.json5 +++ b/rojo-test/serve-tests/ref_forward_sync/src/Workspace/MyModel/init.meta.json5 @@ -1,6 +1,6 @@ { "className": "Model", "attributes": { - "Rojo_Ref_PrimaryPart": "Workspace/MyModel/Target" + "Rojo_Ref_PrimaryPart": "@self/Target" } } diff --git a/rojo-test/serve-tests/ref_two_way_sync/src/Workspace/TestModel/init.meta.json5 b/rojo-test/serve-tests/ref_two_way_sync/src/Workspace/TestModel/init.meta.json5 index 233090f1..2b2f91ba 100644 --- a/rojo-test/serve-tests/ref_two_way_sync/src/Workspace/TestModel/init.meta.json5 +++ b/rojo-test/serve-tests/ref_two_way_sync/src/Workspace/TestModel/init.meta.json5 @@ -1,6 +1,6 @@ { "className": "Model", "attributes": { - "Rojo_Ref_PrimaryPart": "Workspace/TestModel/Part1.model.json5" + "Rojo_Ref_PrimaryPart": "@self/Part1.model.json5" } } diff --git a/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__duplicate_rojo_id-container.model.json5.snap b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__duplicate_rojo_id-container.model.json5.snap index ef2c6d6b..71d41459 100644 --- a/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__duplicate_rojo_id-container.model.json5.snap +++ b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__duplicate_rojo_id-container.model.json5.snap @@ -7,7 +7,7 @@ expression: container.model.json5 { attributes: { Rojo_Id: "value_1", - Rojo_Ref_Value: "ReplicatedStorage/container/value_1.model.json5" + Rojo_Ref_Value: "@self" }, className: "ObjectValue", name: "value_1" @@ -15,7 +15,7 @@ expression: container.model.json5 { attributes: { Rojo_Id: "value_1", - Rojo_Ref_Value: "ReplicatedStorage/container/value_2.model.json5" + Rojo_Ref_Value: "@self" }, className: "ObjectValue", name: "value_2" diff --git a/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties-src__pointer.model.json5.snap b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties-src__pointer.model.json5.snap index 274dadb4..396a91e0 100644 --- a/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties-src__pointer.model.json5.snap +++ b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties-src__pointer.model.json5.snap @@ -4,7 +4,7 @@ expression: src/pointer.model.json5 --- { attributes: { - Rojo_Ref_Value: "target.model.json5" + Rojo_Ref_Value: "@game/target.model.json5" }, className: "ObjectValue" } diff --git a/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties_blank-src__pointer.model.json5.snap b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties_blank-src__pointer.model.json5.snap index 322623e6..135afd1e 100644 --- a/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties_blank-src__pointer.model.json5.snap +++ b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties_blank-src__pointer.model.json5.snap @@ -4,7 +4,7 @@ expression: src/pointer.model.json5 --- { attributes: { - Rojo_Ref_Value: "target.txt" + Rojo_Ref_Value: "@game/target.txt" }, className: "ObjectValue" } diff --git a/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties_conflict-src__Pointer_2.model.json5.snap b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties_conflict-src__Pointer_2.model.json5.snap index 178652af..aadffee8 100644 --- a/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties_conflict-src__Pointer_2.model.json5.snap +++ b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties_conflict-src__Pointer_2.model.json5.snap @@ -4,7 +4,7 @@ expression: src/Pointer_2.model.json5 --- { attributes: { - Rojo_Ref_Value: "Target_2.model.json5" + Rojo_Ref_Value: "@game/Target_2.model.json5" }, className: "ObjectValue" } diff --git a/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties_pruned-src__Pointer1.model.json5.snap b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties_pruned-src__Pointer1.model.json5.snap index 3040e866..53649309 100644 --- a/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties_pruned-src__Pointer1.model.json5.snap +++ b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties_pruned-src__Pointer1.model.json5.snap @@ -4,7 +4,7 @@ expression: src/Pointer1.model.json5 --- { attributes: { - Rojo_Ref_Value: "ReplicatedStorage/Target1.model.json5" + Rojo_Ref_Value: "@game/ReplicatedStorage/Target1.model.json5" }, className: "ObjectValue" } diff --git a/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties_pruned-src__Pointer2.model.json5.snap b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties_pruned-src__Pointer2.model.json5.snap index 9d512230..029c34a6 100644 --- a/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties_pruned-src__Pointer2.model.json5.snap +++ b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties_pruned-src__Pointer2.model.json5.snap @@ -4,7 +4,7 @@ expression: src/Pointer2.model.json5 --- { attributes: { - Rojo_Ref_Value: "ReplicatedStorage/Target2.model.json5" + Rojo_Ref_Value: "@game/ReplicatedStorage/Target2.model.json5" }, className: "ObjectValue" } diff --git a/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties_pruned-src__Pointer3.model.json5.snap b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties_pruned-src__Pointer3.model.json5.snap index b0f55a59..945be246 100644 --- a/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties_pruned-src__Pointer3.model.json5.snap +++ b/rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__ref_properties_pruned-src__Pointer3.model.json5.snap @@ -4,7 +4,7 @@ expression: src/Pointer3.model.json5 --- { attributes: { - Rojo_Ref_Value: "ReplicatedStorage/Target3.model.json5" + Rojo_Ref_Value: "@game/ReplicatedStorage/Target3.model.json5" }, className: "ObjectValue" } diff --git a/rojo-test/syncback-tests/UFOWave_matching/expected/Hitbox/Water.model.json5 b/rojo-test/syncback-tests/UFOWave_matching/expected/Hitbox/Water.model.json5 index 696e6de6..8080d077 100644 --- a/rojo-test/syncback-tests/UFOWave_matching/expected/Hitbox/Water.model.json5 +++ b/rojo-test/syncback-tests/UFOWave_matching/expected/Hitbox/Water.model.json5 @@ -1,6 +1,6 @@ { attributes: { - Rojo_Ref_SoundGroup: "ServerStorage/__server_muted.model.json5" + Rojo_Ref_SoundGroup: "../../__server_muted.model.json5" }, className: "Sound", properties: { diff --git a/rojo-test/syncback-tests/UFOWave_matching/expected/init.meta.json5 b/rojo-test/syncback-tests/UFOWave_matching/expected/init.meta.json5 index 36ad4609..54e6c7f9 100644 --- a/rojo-test/syncback-tests/UFOWave_matching/expected/init.meta.json5 +++ b/rojo-test/syncback-tests/UFOWave_matching/expected/init.meta.json5 @@ -1,6 +1,6 @@ { attributes: { - Rojo_Ref_PrimaryPart: "ServerStorage/UFOWave/TsunamiWave", + Rojo_Ref_PrimaryPart: "@self/TsunamiWave", Speed: 10, ZonesPassed: { Float32: 0 diff --git a/src/change_processor.rs b/src/change_processor.rs index 6838286e..1e95180b 100644 --- a/src/change_processor.rs +++ b/src/change_processor.rs @@ -417,26 +417,25 @@ impl JobThreadContext { /// Uses the `RefPathIndex` for O(affected_files) lookup instead of /// scanning the full tree. After updating files, also updates the index /// keys and filesystem paths so future renames remain efficient. - fn update_ref_paths_after_rename(&self, old_path: &str, new_path: &str) { + fn update_ref_paths_after_rename( + &self, + old_path: &str, + new_path: &str, + tree: &crate::snapshot::RojoTree, + ) { if old_path == new_path { return; } - // Look up affected files via the pre-computed index. let files_from_index = self.ref_path_index.lock().unwrap().find_by_prefix(old_path); if files_from_index.is_empty() { return; } - // The directory may have already been renamed on disk by the time - // this function is called. The index still stores old filesystem - // paths. Remap them: replace the old directory segment with the - // new one so we can find the actual files. let old_segment = old_path.rsplit('/').next().unwrap_or(old_path); let new_segment = new_path.rsplit('/').next().unwrap_or(new_path); - // Pre-compute slugified forms for fallback comparison. let slugified_old = if name_needs_slugify(old_segment) { Some(slugify_name(old_segment)) } else { @@ -457,7 +456,6 @@ impl JobThreadContext { if file_path.exists() { return file_path; } - // File doesn't exist at old path — remap directory component. let mut result = PathBuf::new(); let mut replaced = false; for comp in file_path.components() { @@ -491,13 +489,20 @@ impl JobThreadContext { let mut updated_count = 0; for file_path in &files_to_check { + let source_abs = tree + .get_ids_at_path(file_path) + .first() + .map(|&id| crate::ref_target_path_from_tree(tree, id)) + .unwrap_or_default(); + self.suppress_path(file_path); - match crate::syncback::meta::update_ref_paths_in_file(file_path, old_path, new_path) { + match crate::syncback::meta::update_ref_paths_in_file( + file_path, old_path, new_path, &source_abs, + ) { Ok(true) => { updated_count += 1; } Ok(false) => { - // No stale refs in this file; undo suppression self.unsuppress_path(file_path); } Err(err) => { @@ -1195,7 +1200,9 @@ impl JobThreadContext { crate::ref_target_path_from_tree(&tree, parent_ref); let old_prefix = format!("{}/{}", parent_path, old_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, &tree, + ); } // Fix 1: Update the renamed survivor's in-memory @@ -1805,7 +1812,9 @@ impl JobThreadContext { segment.clone() }; if *old_ref_path != new_ref_path { - self.update_ref_paths_after_rename(old_ref_path, &new_ref_path); + self.update_ref_paths_after_rename( + old_ref_path, &new_ref_path, &tree, + ); } } else if update.changed_name.is_some() || update.changed_class_name.is_some() { // Rename or class change was requested but no filesystem diff --git a/src/rojo_ref.rs b/src/rojo_ref.rs index 35b50bad..8802d03f 100644 --- a/src/rojo_ref.rs +++ b/src/rojo_ref.rs @@ -16,8 +16,9 @@ use serde::{Deserialize, Serialize}; pub const REF_ID_ATTRIBUTE_NAME: &str = "Rojo_Id"; /// Legacy: Pointer attribute prefix using IDs (kept for backwards compatibility) pub const REF_POINTER_ATTRIBUTE_PREFIX: &str = "Rojo_Target_"; -/// Prefix for path-based reference attributes. This is the preferred system -/// that stores the path to the target instance (e.g., "SoundService/Effects"). +/// Prefix for path-based reference attributes using Luau require-by-string +/// style paths (e.g., `@self/Handle.model.json5`, `./Sibling.luau`, +/// `../Uncle/Part.model.json5`, `@game/ReplicatedStorage/Module.luau`). /// No modification to the target instance is needed. pub const REF_PATH_ATTRIBUTE_PREFIX: &str = "Rojo_Ref_"; @@ -35,9 +36,9 @@ pub fn ref_target_attribute_name(prop_name: &str) -> String { format!("{REF_POINTER_ATTRIBUTE_PREFIX}{prop_name}") } -/// Escape "/" in an instance name for use in ref paths. -/// Uses "\/" as escape sequence. Also escapes existing backslashes to "\\" -/// so the escaping is unambiguous. +/// Escape "/" in an instance name for use in legacy instance-name paths. +/// Only used by `ref_target_path` (for debug/log output via `inst_path`). +/// `Rojo_Ref_*` attributes use filesystem names which can't contain `/`. pub fn escape_ref_path_segment(name: &str) -> Cow<'_, str> { if name.contains('/') || name.contains('\\') { Cow::Owned(name.replace('\\', "\\\\").replace('/', "\\/")) @@ -107,15 +108,12 @@ pub fn split_ref_path(path: &str) -> Vec { segments } -/// Compute the path to a target instance for use in a `Rojo_Ref_*` attribute. -/// Returns the slash-separated path from the root (root name excluded). +/// Compute a human-readable path to an instance using raw instance names. +/// Used for debug/log output via `inst_path()`. NOT used for `Rojo_Ref_*` +/// attributes (those use `compute_relative_ref_path` with filesystem names). +/// /// Instance names containing "/" are escaped as "\/" to prevent ambiguity. -/// -/// **Legacy version:** Uses raw instance names with escaping. New code should -/// use `ref_target_path_from_tree()` which uses filesystem names. -/// -/// Example: an instance at `DataModel > Workspace > Part1` returns `"Workspace/Part1"`. -/// An instance named `"A/B"` at Workspace returns `"Workspace/A\/B"`. +/// Example: `DataModel > Workspace > Part1` returns `"Workspace/Part1"`. pub fn ref_target_path(dom: &WeakDom, target_ref: Ref) -> String { let root_ref = dom.root_ref(); let mut components: Vec> = dom @@ -127,21 +125,18 @@ pub fn ref_target_path(dom: &WeakDom, target_ref: Ref) -> String { components.join("/") } -/// Compute the filesystem-name-based path to a target instance for use in -/// `Rojo_Ref_*` attributes. +/// Compute the **absolute** filesystem-name-based path to an instance. +/// +/// Returns a slash-separated path from DataModel's children downward. +/// Used as input to `compute_relative_ref_path()` and for RefPathIndex +/// indexing. Not written to disk directly -- `compute_relative_ref_path` +/// converts absolute paths to the Luau-style relative format. /// /// Each path segment is the **full filesystem name** of the instance: /// - `Folder "Foo"` → `"Foo"` (directory name) /// - `ModuleScript "Foo"` → `"Foo.luau"` (slug + extension) /// - `Script "Foo"` (server) → `"Foo.server.luau"` /// - Dedup'd `ModuleScript "Foo"` → `"Foo~2.luau"` -/// -/// Filesystem names are derived from `instigating_source` paths (the actual -/// file/dir on disk). For instances without filesystem backing (inside .rbxm -/// files, newly added), falls back to the instance name. -/// -/// Since filesystem names can't contain `/` (slugified to `_`), path splitting -/// is a simple `split('/')` -- no escaping needed. pub fn ref_target_path_from_tree(tree: &crate::snapshot::RojoTree, target_ref: Ref) -> String { let dom = tree.inner(); let root_ref = dom.root_ref(); @@ -195,6 +190,108 @@ pub fn ref_target_path_from_tree(tree: &crate::snapshot::RojoTree, target_ref: R components.join("/") } +/// Compute a Luau require-by-string style path from a source instance to a +/// target instance. Both inputs are **absolute** ref paths (slash-separated +/// filesystem names from DataModel's children downward). +/// +/// Returns a prefixed path following Roblox's require-by-string semantics: +/// - `@self` when target == source +/// - `@self/...` when target is a descendant of source +/// - `./...` when source and target share the same parent (1 level up) +/// - `../...` chains for navigation within the same service (2+ levels up) +/// - `@game/...` for cross-service refs or when target is an ancestor of source +/// +/// Reference: +pub fn compute_relative_ref_path(source_abs: &str, target_abs: &str) -> String { + if source_abs == target_abs { + return "@self".to_string(); + } + + let source_prefix = format!("{source_abs}/"); + if target_abs.starts_with(&source_prefix) { + return format!("@self/{}", &target_abs[source_prefix.len()..]); + } + + let source_parts: Vec<&str> = source_abs.split('/').collect(); + let target_parts: Vec<&str> = target_abs.split('/').collect(); + + let common_len = source_parts + .iter() + .zip(target_parts.iter()) + .take_while(|(a, b)| a == b) + .count(); + + if common_len == 0 { + return format!("@game/{target_abs}"); + } + + let ups = source_parts.len() - common_len; + let remaining_parts = &target_parts[common_len..]; + + if remaining_parts.is_empty() { + return format!("@game/{target_abs}"); + } + + let remaining = remaining_parts.join("/"); + + if ups == 1 { + format!("./{remaining}") + } else { + let mut result = String::with_capacity(3 * (ups - 1) + remaining.len()); + for _ in 0..(ups - 1) { + result.push_str("../"); + } + result.push_str(&remaining); + result + } +} + +/// Resolve a relative (Luau-style) ref path back to an absolute ref path +/// using only string manipulation. `source_abs` is the absolute ref path of +/// the instance that owns the attribute. +/// +/// Returns `None` if the path navigates above the root. +pub fn resolve_ref_path_to_absolute(path: &str, source_abs: &str) -> Option { + if let Some(rest) = path.strip_prefix("@game/") { + return Some(rest.to_string()); + } + if path == "@game" { + return Some(String::new()); + } + if path == "@self" { + return Some(source_abs.to_string()); + } + + let (mut parts, rest) = if let Some(rest) = path.strip_prefix("@self/") { + (source_abs.split('/').collect::>(), rest) + } else if let Some(rest) = path.strip_prefix("./") { + let mut p: Vec<&str> = source_abs.split('/').collect(); + p.pop(); + (p, rest) + } else if let Some(rest) = path.strip_prefix("../") { + let mut p: Vec<&str> = source_abs.split('/').collect(); + p.pop(); + if p.pop().is_none() { + return None; + } + (p, rest) + } else { + return Some(path.to_string()); + }; + + for segment in rest.split('/') { + if segment == ".." { + if parts.pop().is_none() { + return None; + } + } else if !segment.is_empty() { + parts.push(segment); + } + } + + Some(parts.join("/")) +} + /// Extract a string value from a Variant that may be String or BinaryString. /// Returns None with a warning for non-string types or invalid UTF-8. /// Used by compute_ref_properties and defer_ref_properties for parsing @@ -300,10 +397,15 @@ impl RefPathIndex { /// Scan all `.meta.json5`, `.model.json5`, `.meta.json`, `.model.json` /// files under `root` for existing `Rojo_Ref_*` attributes and index them. - /// This ensures pre-existing ref attributes (from CLI syncback, manual - /// edits, or previous sessions) are indexed so rename path updates work - /// from the first rename. - pub fn populate_from_dir(&mut self, root: &Path) { + /// + /// Relative paths (prefixed with `@self/`, `./`, `../`) are resolved to + /// absolute using `tree` so the index always stores absolute target paths. + /// This ensures prefix-based lookup works correctly for rename updates. + pub fn populate_from_dir( + &mut self, + root: &Path, + tree: &crate::snapshot::RojoTree, + ) { let mut count = 0usize; let mut dirs_to_visit = vec![root.to_path_buf()]; while let Some(dir) = dirs_to_visit.pop() { @@ -328,13 +430,25 @@ impl RefPathIndex { { continue; } + + let source_abs = tree + .get_ids_at_path(&path) + .first() + .map(|&id| crate::ref_target_path_from_tree(tree, id)) + .unwrap_or_default(); + if let Ok(bytes) = std::fs::read(&path) { if let Ok(val) = crate::json::from_slice::(&bytes) { if let Some(attrs) = val.get("attributes").and_then(|a| a.as_object()) { for (key, value) in attrs { if key.starts_with(crate::REF_PATH_ATTRIBUTE_PREFIX) { if let Some(path_str) = value.as_str() { - self.add(path_str, &path); + let resolved = crate::resolve_ref_path_to_absolute( + path_str, + &source_abs, + ) + .unwrap_or_else(|| path_str.to_string()); + self.add(&resolved, &path); count += 1; } } @@ -712,4 +826,665 @@ mod tests { // Should be deduplicated to just one entry assert_eq!(results.len(), 1); } + + // ----------------------------------------------------------------------- + // compute_relative_ref_path tests -- @self + // ----------------------------------------------------------------------- + + #[test] + fn relative_self_reference() { + assert_eq!( + compute_relative_ref_path("Workspace/Model", "Workspace/Model"), + "@self" + ); + } + + #[test] + fn relative_self_direct_child() { + assert_eq!( + compute_relative_ref_path("Workspace/Model", "Workspace/Model/Handle.model.json5"), + "@self/Handle.model.json5" + ); + } + + #[test] + fn relative_self_nested_descendant() { + assert_eq!( + compute_relative_ref_path( + "Workspace/Model", + "Workspace/Model/SubFolder/Part.model.json5" + ), + "@self/SubFolder/Part.model.json5" + ); + } + + #[test] + fn relative_self_deeply_nested() { + assert_eq!( + compute_relative_ref_path("Workspace/Model", "Workspace/Model/A/B/C/D.luau"), + "@self/A/B/C/D.luau" + ); + } + + #[test] + fn relative_self_tool_handle() { + assert_eq!( + compute_relative_ref_path("Workspace/Tool", "Workspace/Tool/Handle.model.json5"), + "@self/Handle.model.json5" + ); + } + + #[test] + fn relative_self_init_style_child() { + assert_eq!( + compute_relative_ref_path("Workspace/Model", "Workspace/Model/Init"), + "@self/Init" + ); + } + + #[test] + fn relative_self_nested_sound() { + assert_eq!( + compute_relative_ref_path("Workspace/Gun", "Workspace/Gun/Sounds/Fire.model.json5"), + "@self/Sounds/Fire.model.json5" + ); + } + + #[test] + fn relative_self_slugified_child() { + assert_eq!( + compute_relative_ref_path( + "Workspace/Model", + "Workspace/Model/Hey_Bro.server.luau" + ), + "@self/Hey_Bro.server.luau" + ); + } + + #[test] + fn relative_self_dedup_child() { + assert_eq!( + compute_relative_ref_path("Workspace/Model", "Workspace/Model/Data~2"), + "@self/Data~2" + ); + } + + // ----------------------------------------------------------------------- + // compute_relative_ref_path tests -- ./ + // ----------------------------------------------------------------------- + + #[test] + fn relative_dot_sibling_script() { + assert_eq!( + compute_relative_ref_path( + "Workspace/Folder/ScriptA.server.luau", + "Workspace/Folder/ScriptB.server.luau" + ), + "./ScriptB.server.luau" + ); + } + + #[test] + fn relative_dot_sibling_module() { + assert_eq!( + compute_relative_ref_path( + "Workspace/Folder/ScriptA.server.luau", + "Workspace/Folder/Config.luau" + ), + "./Config.luau" + ); + } + + #[test] + fn relative_dot_sibling_directory() { + assert_eq!( + compute_relative_ref_path( + "Workspace/Folder/ScriptA.server.luau", + "Workspace/Folder/SubFolder" + ), + "./SubFolder" + ); + } + + #[test] + fn relative_dot_sibling_then_descend() { + assert_eq!( + compute_relative_ref_path( + "Workspace/Folder/ScriptA.server.luau", + "Workspace/Folder/SubFolder/Deep.luau" + ), + "./SubFolder/Deep.luau" + ); + } + + #[test] + fn relative_dot_object_value_sibling() { + assert_eq!( + compute_relative_ref_path( + "Workspace/Folder/ObjectValue.model.json5", + "Workspace/Folder/Target.model.json5" + ), + "./Target.model.json5" + ); + } + + #[test] + fn relative_dot_beam_attachment0() { + assert_eq!( + compute_relative_ref_path( + "Workspace/Beams/Beam.model.json5", + "Workspace/Beams/Att1.model.json5" + ), + "./Att1.model.json5" + ); + } + + #[test] + fn relative_dot_beam_attachment1() { + assert_eq!( + compute_relative_ref_path( + "Workspace/Beams/Beam.model.json5", + "Workspace/Beams/Att2.model.json5" + ), + "./Att2.model.json5" + ); + } + + #[test] + fn relative_dot_slugified_sibling() { + assert_eq!( + compute_relative_ref_path( + "Workspace/Folder/A.luau", + "Workspace/Folder/Hey_Bro.server.luau" + ), + "./Hey_Bro.server.luau" + ); + } + + #[test] + fn relative_dot_dedup_sibling_folder() { + assert_eq!( + compute_relative_ref_path("Workspace/Folder/A.luau", "Workspace/Folder/Data~2"), + "./Data~2" + ); + } + + #[test] + fn relative_dot_dedup_sibling_model() { + assert_eq!( + compute_relative_ref_path( + "Workspace/Folder/A.luau", + "Workspace/Folder/Data~3.model.json5" + ), + "./Data~3.model.json5" + ); + } + + // ----------------------------------------------------------------------- + // compute_relative_ref_path tests -- ../ + // ----------------------------------------------------------------------- + + #[test] + fn relative_dotdot_2_ups() { + assert_eq!( + compute_relative_ref_path( + "Workspace/A/Script.server.luau", + "Workspace/B/Part.model.json5" + ), + "../B/Part.model.json5" + ); + } + + #[test] + fn relative_dotdot_2_ups_directory() { + assert_eq!( + compute_relative_ref_path("Workspace/A/Script.server.luau", "Workspace/B"), + "../B" + ); + } + + #[test] + fn relative_dotdot_3_ups() { + assert_eq!( + compute_relative_ref_path( + "Workspace/A/B/Script.server.luau", + "Workspace/C/Part.model.json5" + ), + "../../C/Part.model.json5" + ); + } + + #[test] + fn relative_dotdot_4_ups() { + assert_eq!( + compute_relative_ref_path( + "Workspace/A/B/C/Script.luau", + "Workspace/D/E/Part.model.json5" + ), + "../../../D/E/Part.model.json5" + ); + } + + #[test] + fn relative_dotdot_5_ups() { + assert_eq!( + compute_relative_ref_path( + "Workspace/A/B/C/D/Script.luau", + "Workspace/E/Part.model.json5" + ), + "../../../../E/Part.model.json5" + ); + } + + #[test] + fn relative_dotdot_peer_model() { + assert_eq!( + compute_relative_ref_path( + "Workspace/Models/Car/Body.model.json5", + "Workspace/Models/Truck/Body.model.json5" + ), + "../Truck/Body.model.json5" + ); + } + + #[test] + fn relative_dotdot_cousin() { + assert_eq!( + compute_relative_ref_path("Workspace/A/Deep/Script.luau", "Workspace/A/Other.luau"), + "../Other.luau" + ); + } + + #[test] + fn relative_dotdot_beam_uncle_folder() { + assert_eq!( + compute_relative_ref_path( + "Workspace/A/B/Beam.model.json5", + "Workspace/A/C/Att.model.json5" + ), + "../C/Att.model.json5" + ); + } + + #[test] + fn relative_dotdot_cross_system() { + assert_eq!( + compute_relative_ref_path( + "Workspace/Systems/Combat/Hitbox.server.luau", + "Workspace/Systems/Audio/HitSound.model.json5" + ), + "../Audio/HitSound.model.json5" + ); + } + + #[test] + fn relative_dotdot_same_zone() { + assert_eq!( + compute_relative_ref_path( + "Workspace/Map/Zone1/Spawns/Spawn1.model.json5", + "Workspace/Map/Zone1/Props/Tree.model.json5" + ), + "../Props/Tree.model.json5" + ); + } + + #[test] + fn relative_dotdot_cross_zone() { + assert_eq!( + compute_relative_ref_path( + "Workspace/Map/Zone1/Spawns/Spawn1.model.json5", + "Workspace/Map/Zone2/Flag.model.json5" + ), + "../../Zone2/Flag.model.json5" + ); + } + + // ----------------------------------------------------------------------- + // compute_relative_ref_path tests -- @game/ + // ----------------------------------------------------------------------- + + #[test] + fn relative_game_cross_service_classic() { + assert_eq!( + compute_relative_ref_path( + "Workspace/Script.server.luau", + "ReplicatedStorage/Module.luau" + ), + "@game/ReplicatedStorage/Module.luau" + ); + } + + #[test] + fn relative_game_to_server_storage() { + assert_eq!( + compute_relative_ref_path("Workspace/Script.server.luau", "ServerStorage/Data.luau"), + "@game/ServerStorage/Data.luau" + ); + } + + #[test] + fn relative_game_sss_to_rs() { + assert_eq!( + compute_relative_ref_path( + "ServerScriptService/Main.server.luau", + "ReplicatedStorage/Shared/Utils.luau" + ), + "@game/ReplicatedStorage/Shared/Utils.luau" + ); + } + + #[test] + fn relative_game_client_to_rs() { + assert_eq!( + compute_relative_ref_path( + "StarterGui/ScreenGui/Button.client.luau", + "ReplicatedStorage/UI/Theme.luau" + ), + "@game/ReplicatedStorage/UI/Theme.luau" + ); + } + + #[test] + fn relative_game_to_service_itself() { + assert_eq!( + compute_relative_ref_path("Workspace/A/B/C/Script.luau", "Lighting"), + "@game/Lighting" + ); + } + + #[test] + fn relative_game_ancestor_own_service() { + assert_eq!( + compute_relative_ref_path("Workspace/A/B/C/Script.luau", "Workspace"), + "@game/Workspace" + ); + } + + #[test] + fn relative_game_ancestor_grandparent() { + assert_eq!( + compute_relative_ref_path("Workspace/A/B/Script.luau", "Workspace/A"), + "@game/Workspace/A" + ); + } + + #[test] + fn relative_game_ancestor_great_grandparent() { + assert_eq!( + compute_relative_ref_path("Workspace/A/B/C/Script.luau", "Workspace/A/B"), + "@game/Workspace/A/B" + ); + } + + #[test] + fn relative_game_ancestor_own_service_single_level() { + assert_eq!( + compute_relative_ref_path("ReplicatedStorage/A.luau", "ReplicatedStorage"), + "@game/ReplicatedStorage" + ); + } + + #[test] + fn relative_game_to_sound_service() { + assert_eq!( + compute_relative_ref_path("Workspace/Script.luau", "SoundService/BGM.model.json5"), + "@game/SoundService/BGM.model.json5" + ); + } + + // ----------------------------------------------------------------------- + // compute_relative_ref_path tests -- special names + // ----------------------------------------------------------------------- + + #[test] + fn relative_special_windows_reserved() { + assert_eq!( + compute_relative_ref_path("Workspace/Folder/A.luau", "Workspace/Other/CON_.luau"), + "../Other/CON_.luau" + ); + } + + #[test] + fn relative_special_into_dedup_folder() { + assert_eq!( + compute_relative_ref_path("Workspace/A.luau", "Workspace/Data~2/Child.luau"), + "./Data~2/Child.luau" + ); + } + + #[test] + fn relative_special_init_style_child() { + assert_eq!( + compute_relative_ref_path("Workspace/Model", "Workspace/Model/Scripts"), + "@self/Scripts" + ); + } + + #[test] + fn relative_special_init_style_both() { + assert_eq!( + compute_relative_ref_path("Workspace/Scripts", "Workspace/Scripts/Foo"), + "@self/Foo" + ); + } + + // ----------------------------------------------------------------------- + // compute_relative_ref_path tests -- tricky edge cases + // ----------------------------------------------------------------------- + + #[test] + fn relative_edge_service_own_child() { + assert_eq!( + compute_relative_ref_path("Workspace", "Workspace/Model.model.json5"), + "@self/Model.model.json5" + ); + } + + #[test] + fn relative_edge_service_to_service() { + assert_eq!( + compute_relative_ref_path("Workspace", "ReplicatedStorage/Module.luau"), + "@game/ReplicatedStorage/Module.luau" + ); + } + + #[test] + fn relative_edge_service_self_ref() { + assert_eq!( + compute_relative_ref_path("Workspace", "Workspace"), + "@self" + ); + } + + #[test] + fn relative_edge_service_to_different_service() { + assert_eq!( + compute_relative_ref_path("Workspace", "Lighting"), + "@game/Lighting" + ); + } + + #[test] + fn relative_edge_segment_prefix_ambiguity() { + assert_eq!( + compute_relative_ref_path("Workspace/A", "Workspace/AB"), + "./AB" + ); + } + + #[test] + fn relative_edge_same_stem_different_ext() { + assert_eq!( + compute_relative_ref_path("Workspace/Foo.server.luau", "Workspace/Foo.luau"), + "./Foo.luau" + ); + } + + #[test] + fn relative_edge_deep_close_cousins() { + assert_eq!( + compute_relative_ref_path( + "Workspace/A/B/C/D/E/F/Script.luau", + "Workspace/A/B/C/D/E/G/Target.luau" + ), + "../G/Target.luau" + ); + } + + #[test] + fn relative_edge_deep_far_apart() { + assert_eq!( + compute_relative_ref_path( + "Workspace/A/B/C/D/E/F/Script.luau", + "Workspace/X/Target.luau" + ), + "../../../../../../X/Target.luau" + ); + } + + // ----------------------------------------------------------------------- + // resolve_ref_path_to_absolute tests + // ----------------------------------------------------------------------- + + #[test] + fn resolve_abs_game_prefix() { + assert_eq!( + resolve_ref_path_to_absolute("@game/ReplicatedStorage/Module.luau", "Workspace/Script"), + Some("ReplicatedStorage/Module.luau".to_string()) + ); + } + + #[test] + fn resolve_abs_self_no_path() { + assert_eq!( + resolve_ref_path_to_absolute("@self", "Workspace/Model"), + Some("Workspace/Model".to_string()) + ); + } + + #[test] + fn resolve_abs_self_with_child() { + assert_eq!( + resolve_ref_path_to_absolute("@self/Handle.model.json5", "Workspace/Model"), + Some("Workspace/Model/Handle.model.json5".to_string()) + ); + } + + #[test] + fn resolve_abs_self_nested() { + assert_eq!( + resolve_ref_path_to_absolute("@self/A/B/C", "Workspace/Model"), + Some("Workspace/Model/A/B/C".to_string()) + ); + } + + #[test] + fn resolve_abs_dot_sibling() { + assert_eq!( + resolve_ref_path_to_absolute("./Sibling.luau", "Workspace/Folder/Script.luau"), + Some("Workspace/Folder/Sibling.luau".to_string()) + ); + } + + #[test] + fn resolve_abs_dot_sibling_descend() { + assert_eq!( + resolve_ref_path_to_absolute("./Sub/Deep.luau", "Workspace/Folder/Script.luau"), + Some("Workspace/Folder/Sub/Deep.luau".to_string()) + ); + } + + #[test] + fn resolve_abs_dotdot_2_ups() { + assert_eq!( + resolve_ref_path_to_absolute("../B/Part.model.json5", "Workspace/A/Script.luau"), + Some("Workspace/B/Part.model.json5".to_string()) + ); + } + + #[test] + fn resolve_abs_dotdot_3_ups() { + assert_eq!( + resolve_ref_path_to_absolute("../../C/Part.model.json5", "Workspace/A/B/Script.luau"), + Some("Workspace/C/Part.model.json5".to_string()) + ); + } + + #[test] + fn resolve_abs_dotdot_cross_service_via_root() { + assert_eq!( + resolve_ref_path_to_absolute( + "../../ReplicatedStorage/Module.luau", + "Workspace/A/Script.luau" + ), + Some("ReplicatedStorage/Module.luau".to_string()) + ); + } + + #[test] + fn resolve_abs_bare_path_legacy() { + assert_eq!( + resolve_ref_path_to_absolute( + "Workspace/Model/Handle.model.json5", + "anything/here" + ), + Some("Workspace/Model/Handle.model.json5".to_string()) + ); + } + + #[test] + fn resolve_abs_dotdot_mid_path() { + assert_eq!( + resolve_ref_path_to_absolute("@self/Sub/../Handle.model.json5", "Workspace/Model"), + Some("Workspace/Model/Handle.model.json5".to_string()) + ); + } + + #[test] + fn resolve_abs_dotdot_above_root_returns_none() { + assert_eq!( + resolve_ref_path_to_absolute("../../X", "Workspace"), + None + ); + } + + #[test] + fn resolve_then_compute_round_trip() { + let source = "Workspace/A/B/Script.luau"; + let target = "Workspace/C/Part.model.json5"; + let relative = compute_relative_ref_path(source, target); + assert_eq!(relative, "../../C/Part.model.json5"); + let resolved = resolve_ref_path_to_absolute(&relative, source).unwrap(); + assert_eq!(resolved, target); + } + + #[test] + fn resolve_then_compute_round_trip_self() { + let source = "Workspace/Model"; + let target = "Workspace/Model/Handle.model.json5"; + let relative = compute_relative_ref_path(source, target); + assert_eq!(relative, "@self/Handle.model.json5"); + let resolved = resolve_ref_path_to_absolute(&relative, source).unwrap(); + assert_eq!(resolved, target); + } + + #[test] + fn resolve_then_compute_round_trip_game() { + let source = "Workspace/Script.luau"; + let target = "ReplicatedStorage/Module.luau"; + let relative = compute_relative_ref_path(source, target); + assert_eq!(relative, "@game/ReplicatedStorage/Module.luau"); + let resolved = resolve_ref_path_to_absolute(&relative, source).unwrap(); + assert_eq!(resolved, target); + } + + #[test] + fn resolve_then_compute_round_trip_dot() { + let source = "Workspace/Folder/A.luau"; + let target = "Workspace/Folder/B.luau"; + let relative = compute_relative_ref_path(source, target); + assert_eq!(relative, "./B.luau"); + let resolved = resolve_ref_path_to_absolute(&relative, source).unwrap(); + assert_eq!(resolved, target); + } } diff --git a/src/serve_session.rs b/src/serve_session.rs index d67f7604..3e5c5c2a 100644 --- a/src/serve_session.rs +++ b/src/serve_session.rs @@ -176,7 +176,9 @@ impl ServeSession { let suppressed_paths = Arc::new(Mutex::new(std::collections::HashMap::new())); let ref_path_index = { let mut index = crate::RefPathIndex::new(); - index.populate_from_dir(root_project.folder_location()); + let tree_guard = tree.lock().unwrap(); + index.populate_from_dir(root_project.folder_location(), &tree_guard); + drop(tree_guard); Arc::new(Mutex::new(index)) }; diff --git a/src/snapshot/patch_apply.rs b/src/snapshot/patch_apply.rs index f8a1031b..30c483af 100644 --- a/src/snapshot/patch_apply.rs +++ b/src/snapshot/patch_apply.rs @@ -144,7 +144,7 @@ fn finalize_patch_application(context: PatchApplyContext, tree: &mut RojoTree) - let mut path_rewrites = Vec::new(); for (id, map) in context.path_refs_to_rewrite { for (prop_name, path) in map { - if let Some(target) = tree.get_instance_by_path(&path) { + if let Some(target) = tree.resolve_ref_path(&path, id) { log::debug!( "Resolved path reference {} -> {} (path: {})", prop_name, diff --git a/src/snapshot/patch_compute.rs b/src/snapshot/patch_compute.rs index 49acf0ed..3e9268ad 100644 --- a/src/snapshot/patch_compute.rs +++ b/src/snapshot/patch_compute.rs @@ -104,7 +104,7 @@ fn compute_property_patches( let mut visited_properties = UstrSet::default(); let mut changed_properties = UstrMap::new(); - let attribute_ref_properties = compute_ref_properties(snapshot, tree); + let attribute_ref_properties = compute_ref_properties(snapshot, tree, instance.id()); let changed_name = if snapshot.name == instance.name() { None @@ -251,6 +251,7 @@ fn compute_children_patches( fn compute_ref_properties( snapshot: &InstanceSnapshot, tree: &RojoTree, + source_ref: Ref, ) -> UstrMap> { let mut map = UstrMap::new(); let attributes = match snapshot.properties.get(&ustr("Attributes")) { @@ -290,7 +291,7 @@ fn compute_ref_properties( let Some(path) = crate::variant_as_str(attr_value, attr_name) else { continue; }; - if let Some(target_id) = tree.get_instance_by_path(path) { + if let Some(target_id) = tree.resolve_ref_path(path, source_ref) { map.insert(ustr(prop_name), Some(Variant::Ref(target_id))); } else { // Path not found yet - will be resolved during patch apply diff --git a/src/snapshot/tree.rs b/src/snapshot/tree.rs index c66ecd30..fd7922b7 100644 --- a/src/snapshot/tree.rs +++ b/src/snapshot/tree.rs @@ -285,6 +285,109 @@ impl RojoTree { Some(current_ref) } + /// Resolve a Luau require-by-string style ref path to a target instance. + /// + /// `source_ref` is the instance that owns the `Rojo_Ref_*` attribute. + /// Prefix semantics (per Roblox require-by-string): + /// - `@game/rest` -- resolve from DataModel root + /// - `@self` -- return source_ref + /// - `@self/rest` -- resolve from source_ref + /// - `./rest` -- resolve from source_ref's parent + /// - `../rest` -- resolve from source_ref's grandparent + /// - bare path (no prefix) -- legacy fallback, resolve like `@game/` + /// + /// During segment walk, `..` navigates to the parent. + pub fn resolve_ref_path(&self, path: &str, source_ref: Ref) -> Option { + if let Some(rest) = path.strip_prefix("@game/") { + return self.walk_segments(self.get_root_id(), rest); + } + if path == "@game" { + return Some(self.get_root_id()); + } + if path == "@self" { + return Some(source_ref); + } + if let Some(rest) = path.strip_prefix("@self/") { + return self.walk_segments(source_ref, rest); + } + if let Some(rest) = path.strip_prefix("./") { + let parent = self.inner.get_by_ref(source_ref)?.parent(); + if parent.is_none() { + return None; + } + return self.walk_segments(parent, rest); + } + if let Some(rest) = path.strip_prefix("../") { + let parent = self.inner.get_by_ref(source_ref)?.parent(); + if parent.is_none() { + return None; + } + let grandparent = self.inner.get_by_ref(parent)?.parent(); + if grandparent.is_none() { + return None; + } + return self.walk_segments(grandparent, rest); + } + + // Bare path (no prefix) -- legacy backward compatibility + self.walk_segments(self.get_root_id(), path) + } + + /// Walk `/`-separated segments from a starting Ref, handling `..` as + /// "go to parent". Each non-`..` segment matches a child by filesystem + /// name (case-insensitive), falling back to instance name. + fn walk_segments(&self, start: Ref, rest: &str) -> Option { + if rest.is_empty() { + return Some(start); + } + + let mut current_ref = start; + for segment in rest.split('/') { + if segment.is_empty() { + continue; + } + if segment == ".." { + let inst = self.inner.get_by_ref(current_ref)?; + let parent = inst.parent(); + if parent.is_none() { + return None; + } + current_ref = parent; + continue; + } + + let current = self.inner.get_by_ref(current_ref)?; + let children = current.children(); + + let mut found = false; + for &child_ref in children { + let fs_name = self.filesystem_name_for(child_ref); + if fs_name.eq_ignore_ascii_case(segment) { + current_ref = child_ref; + found = true; + break; + } + } + + if !found { + for &child_ref in children { + let child = self.inner.get_by_ref(child_ref)?; + if child.name.eq_ignore_ascii_case(segment) { + current_ref = child_ref; + found = true; + break; + } + } + } + + if !found { + return None; + } + } + + Some(current_ref) + } + /// Returns the filesystem name for an instance (the filename or directory /// name on disk). Derived from `instigating_source` if available, otherwise /// falls back to the instance name. diff --git a/src/syncback/fs_snapshot.rs b/src/syncback/fs_snapshot.rs index 451d13d3..e787737e 100644 --- a/src/syncback/fs_snapshot.rs +++ b/src/syncback/fs_snapshot.rs @@ -535,7 +535,7 @@ mod tests { meta_content.as_bytes().to_vec(), ); - let substitutions = vec![("Workspace/Foo".to_string(), "Workspace/Foo~2".to_string())]; + let substitutions = vec![("Workspace/Foo".to_string(), "./Foo~2".to_string())]; snap.fix_ref_paths(&substitutions); let result = std::str::from_utf8( @@ -546,7 +546,7 @@ mod tests { .unwrap(); assert!( - result.contains(r#""Rojo_Ref_PrimaryPart": "Workspace/Foo~2""#), + result.contains(r#""Rojo_Ref_PrimaryPart": "./Foo~2""#), "Rojo_Ref line should be updated. Got:\n{}", result ); @@ -566,7 +566,7 @@ mod tests { b"-- Rojo_Ref_PrimaryPart Workspace/Foo".to_vec(), ); - let substitutions = vec![("Workspace/Foo".to_string(), "Workspace/Foo~2".to_string())]; + let substitutions = vec![("Workspace/Foo".to_string(), "./Foo~2".to_string())]; snap.fix_ref_paths(&substitutions); let result = std::str::from_utf8( @@ -581,7 +581,7 @@ mod tests { result ); assert!( - !result.contains("Workspace/Foo~2"), + !result.contains("./Foo~2"), "Non-meta file should not have substitution applied" ); } @@ -618,11 +618,11 @@ mod tests { let substitutions = vec![ ( "__ROJO_REF_00000000000000000000000000000001__".to_string(), - "Beams1/001~5".to_string(), + "./001~5".to_string(), ), ( "__ROJO_REF_00000000000000000000000000000002__".to_string(), - "Beams1/002~7.model.json5".to_string(), + "./002~7.model.json5".to_string(), ), ]; snap.fix_ref_paths(&substitutions); @@ -635,11 +635,11 @@ mod tests { .unwrap(); assert!( - result.contains(r#""Rojo_Ref_Attachment0": "Beams1/001~5""#), + result.contains(r#""Rojo_Ref_Attachment0": "./001~5""#), "Attachment0 should resolve to 001~5. Got:\n{result}" ); assert!( - result.contains(r#""Rojo_Ref_Attachment1": "Beams1/002~7.model.json5""#), + result.contains(r#""Rojo_Ref_Attachment1": "./002~7.model.json5""#), "Attachment1 should resolve to 002~7. Got:\n{result}" ); assert!( @@ -715,15 +715,15 @@ mod tests { let substitutions = vec![ ( "__ROJO_REF_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa__".to_string(), - "Beams1/001~2".to_string(), + "./001~2".to_string(), ), ( "__ROJO_REF_bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb__".to_string(), - "Beams1/001~5".to_string(), + "./001~5".to_string(), ), ( "__ROJO_REF_cccccccccccccccccccccccccccccccc__".to_string(), - "Workspace/Model/Handle.model.json5".to_string(), + "@self/Handle.model.json5".to_string(), ), ]; snap.fix_ref_paths(&substitutions); @@ -736,15 +736,15 @@ mod tests { .unwrap(); assert!( - result.contains(r#""Rojo_Ref_Attachment0": "Beams1/001~2""#), + result.contains(r#""Rojo_Ref_Attachment0": "./001~2""#), "Attachment0 wrong. Got:\n{result}" ); assert!( - result.contains(r#""Rojo_Ref_Attachment1": "Beams1/001~5""#), + result.contains(r#""Rojo_Ref_Attachment1": "./001~5""#), "Attachment1 wrong. Got:\n{result}" ); assert!( - result.contains(r#""Rojo_Ref_PrimaryPart": "Workspace/Model/Handle.model.json5""#), + result.contains(r#""Rojo_Ref_PrimaryPart": "@self/Handle.model.json5""#), "PrimaryPart wrong. Got:\n{result}" ); } diff --git a/src/syncback/meta.rs b/src/syncback/meta.rs index 4dc969c5..6d50b714 100644 --- a/src/syncback/meta.rs +++ b/src/syncback/meta.rs @@ -96,10 +96,17 @@ pub fn remove_model_name(model_path: &Path) -> anyhow::Result /// For each attribute whose key starts with `Rojo_Ref_` and whose string /// value starts with `old_prefix`, replaces the prefix with `new_prefix`. /// Returns true if any attribute was updated. +/// Update `Rojo_Ref_*` attributes in a meta/model file after a rename. +/// +/// For each attribute, resolves the on-disk relative path to absolute using +/// `source_abs`, checks if the resolved absolute path is affected by the +/// rename (`old_prefix` → `new_prefix`), and if so, recomputes the relative +/// path from `source_abs` to the new absolute target. pub fn update_ref_paths_in_file( file_path: &Path, old_prefix: &str, new_prefix: &str, + source_abs: &str, ) -> anyhow::Result { use crate::REF_PATH_ATTRIBUTE_PREFIX; @@ -118,17 +125,28 @@ pub fn update_ref_paths_in_file( ); } + let old_prefix_slash = format!("{old_prefix}/"); let mut updated = false; if let Some(attrs) = val.get_mut("attributes").and_then(|a| a.as_object_mut()) { for (key, value) in attrs.iter_mut() { - if key.starts_with(REF_PATH_ATTRIBUTE_PREFIX) { - if let Some(path_str) = value.as_str() { - if path_str == old_prefix || path_str.starts_with(&format!("{old_prefix}/")) { - let new_path = format!("{new_prefix}{}", &path_str[old_prefix.len()..]); - *value = serde_json::Value::String(new_path); - updated = true; - } - } + if !key.starts_with(REF_PATH_ATTRIBUTE_PREFIX) { + continue; + } + let Some(path_str) = value.as_str() else { + continue; + }; + let Some(resolved) = + crate::resolve_ref_path_to_absolute(path_str, source_abs) + else { + continue; + }; + if resolved == old_prefix || resolved.starts_with(&old_prefix_slash) { + let new_abs = + format!("{new_prefix}{}", &resolved[old_prefix.len()..]); + let new_relative = + crate::compute_relative_ref_path(source_abs, &new_abs); + *value = serde_json::Value::String(new_relative); + updated = true; } } } diff --git a/src/syncback/mod.rs b/src/syncback/mod.rs index 666f4fb7..fa141e0f 100644 --- a/src/syncback/mod.rs +++ b/src/syncback/mod.rs @@ -175,7 +175,8 @@ pub fn syncback_loop_with_stats( log::debug!("Collecting referents for new DOM..."); let mut deferred_referents = collect_referents(&new_tree, &pre_prune_paths, None); - let placeholder_map = std::mem::take(&mut deferred_referents.placeholder_to_target); + let placeholder_map = + std::mem::take(&mut deferred_referents.placeholder_to_source_and_target); // Remove any properties that are manually blocked from syncback via the // project file. @@ -622,12 +623,17 @@ pub fn syncback_loop_with_stats( let final_map = ref_path_map.borrow(); let mut substitutions: Vec<(String, String)> = Vec::new(); - for (placeholder, target_ref) in &placeholder_map { - let final_path = final_map + for (placeholder, (source_ref, target_ref)) in &placeholder_map { + let source_abs = final_map + .get(source_ref) + .cloned() + .unwrap_or_else(|| tentative_fs_path_public(&new_tree, *source_ref)); + let target_abs = final_map .get(target_ref) .cloned() .unwrap_or_else(|| tentative_fs_path_public(&new_tree, *target_ref)); - substitutions.push((placeholder.clone(), final_path)); + let relative = crate::compute_relative_ref_path(&source_abs, &target_abs); + substitutions.push((placeholder.clone(), relative)); } if !substitutions.is_empty() { log::debug!( diff --git a/src/syncback/ref_properties.rs b/src/syncback/ref_properties.rs index a3a09a2d..0985e266 100644 --- a/src/syncback/ref_properties.rs +++ b/src/syncback/ref_properties.rs @@ -142,11 +142,11 @@ pub struct RefLinks { id_links: HashMap>, /// Target instances that need a Rojo_Id written (for ID-based system). targets_needing_id: HashSet, - /// Maps placeholder strings to target Refs. Populated when `final_paths` - /// is not available (CLI syncback). After the syncback walk populates the - /// `ref_path_map`, these placeholders are substituted with the correct - /// dedup-aware filesystem paths via `fix_ref_paths`. - pub placeholder_to_target: HashMap, + /// Maps placeholder strings to (source_ref, target_ref) pairs. Populated + /// when `final_paths` is not available (CLI syncback pre-walk). The + /// placeholder encodes both source and target so that post-processing can + /// compute the correct relative path for each (source, target) pair. + pub placeholder_to_source_and_target: HashMap, } #[derive(PartialEq, Eq)] @@ -195,7 +195,7 @@ pub fn collect_referents( let mut path_links: HashMap> = HashMap::new(); let id_links: HashMap> = HashMap::new(); let targets_needing_id: HashSet = HashSet::new(); - let mut placeholder_to_target: HashMap = HashMap::new(); + let mut placeholder_to_source_and_target: HashMap = HashMap::new(); let mut queue = VecDeque::new(); queue.push_back(dom.root_ref()); @@ -213,35 +213,36 @@ pub fn collect_referents( } if dom.get_by_ref(*target_ref).is_some() { - // Target exists in DOM -- use path-based system. - let target_path = if let Some(fp) = final_paths { - // final_paths available (dedup-aware) -- use directly. - fp.get(target_ref) + let ref_path = if let Some(fp) = final_paths { + let source_abs = fp + .get(&inst_ref) .cloned() - .unwrap_or_else(|| tentative_fs_path(dom, *target_ref)) + .unwrap_or_else(|| tentative_fs_path(dom, inst_ref)); + let target_abs = fp + .get(target_ref) + .cloned() + .unwrap_or_else(|| tentative_fs_path(dom, *target_ref)); + crate::compute_relative_ref_path(&source_abs, &target_abs) } else { - // No final_paths yet (pre-walk). Use a unique placeholder - // keyed by the target Ref so that duplicate-named siblings - // get distinct values. The syncback post-processing step - // substitutes these with the correct dedup'd paths. - let placeholder = ref_placeholder(*target_ref); - placeholder_to_target.insert(placeholder.clone(), *target_ref); + let placeholder = ref_placeholder(inst_ref, *target_ref); + placeholder_to_source_and_target + .insert(placeholder.clone(), (inst_ref, *target_ref)); placeholder }; path_links.entry(inst_ref).or_default().push(PathRefLink { name: *prop_name, - path: target_path, + path: ref_path, }); } else if let Some(external_path) = pre_prune_paths.get(target_ref) { log::debug!( - "Property {}.{} points to pruned instance at '{}', storing as path reference", + "Property {}.{} points to pruned instance at '{}', storing as @game/ reference", tentative_fs_path(dom, inst_ref), prop_name, external_path ); path_links.entry(inst_ref).or_default().push(PathRefLink { name: *prop_name, - path: external_path.clone(), + path: format!("@game/{external_path}"), }); } else { log::warn!( @@ -257,19 +258,16 @@ pub fn collect_referents( path_links, id_links, targets_needing_id, - placeholder_to_target, + placeholder_to_source_and_target, } } -/// Prefix used for ref placeholder strings. Chosen to be unambiguous with -/// real filesystem paths (which cannot contain double underscores in this -/// position). const REF_PLACEHOLDER_PREFIX: &str = "__ROJO_REF_"; const REF_PLACEHOLDER_SUFFIX: &str = "__"; -/// Build a unique placeholder string for a target Ref. -fn ref_placeholder(target: Ref) -> String { - format!("{REF_PLACEHOLDER_PREFIX}{target}{REF_PLACEHOLDER_SUFFIX}") +/// Build a unique placeholder string encoding both source and target Refs. +fn ref_placeholder(source: Ref, target: Ref) -> String { + format!("{REF_PLACEHOLDER_PREFIX}{source}_TO_{target}{REF_PLACEHOLDER_SUFFIX}") } /// Writes reference attributes to instances in the DOM. @@ -453,7 +451,7 @@ mod tests { } #[test] - fn collect_referents_uses_unique_placeholders_for_duplicate_names() { + fn collect_referents_uses_source_aware_placeholders() { let (dom, _, attachments, beam) = make_beam_attachment_dom(); let links = collect_referents(&dom, &HashMap::new(), None); @@ -466,16 +464,21 @@ mod tests { path.starts_with(REF_PLACEHOLDER_PREFIX), "path should be a placeholder, got: {path}" ); - - assert_eq!( - links.placeholder_to_target.get(path).copied(), - Some(attachments[2]), - "placeholder should map to the specific target Ref" + assert!( + path.contains("_TO_"), + "placeholder should encode both source and target, got: {path}" ); + + let (src, tgt) = links + .placeholder_to_source_and_target + .get(path) + .expect("placeholder should map to (source, target)"); + assert_eq!(*src, beam); + assert_eq!(*tgt, attachments[2]); } #[test] - fn placeholders_are_unique_per_target_ref() { + fn placeholders_are_unique_per_source_target_pair() { let mut dom2 = WeakDom::new(InstanceBuilder::new("DataModel")); let root2 = dom2.root_ref(); let c2 = dom2.insert(root2, InstanceBuilder::new("Folder").with_name("Beams1")); @@ -491,14 +494,19 @@ mod tests { let links = collect_referents(&dom2, &HashMap::new(), None); - let placeholders: Vec<&String> = links.placeholder_to_target.keys().collect(); + let placeholders: Vec<&String> = + links.placeholder_to_source_and_target.keys().collect(); assert!( placeholders.len() >= 2, "should have at least 2 placeholders" ); - let targets: HashSet<&Ref> = links.placeholder_to_target.values().collect(); + let targets: HashSet = links + .placeholder_to_source_and_target + .values() + .map(|(_, t)| *t) + .collect(); assert!( targets.contains(&att_a), "should have placeholder for att_a" @@ -508,7 +516,6 @@ mod tests { "should have placeholder for att_b" ); - // Crucially: the placeholder strings themselves must be distinct let unique_placeholders: HashSet<&String> = placeholders.iter().copied().collect(); assert_eq!( unique_placeholders.len(), @@ -518,20 +525,24 @@ mod tests { } #[test] - fn collect_referents_with_final_paths_skips_placeholders() { + fn collect_referents_with_final_paths_produces_relative_paths() { let (dom, _, attachments, beam) = make_beam_attachment_dom(); let mut final_paths = HashMap::new(); - final_paths.insert(attachments[2], "Beams1/001~3".to_string()); + final_paths.insert(beam, "Beams1/BeamA.model.json5".to_string()); + final_paths.insert(attachments[2], "Beams1/001~3.model.json5".to_string()); let links = collect_referents(&dom, &HashMap::new(), Some(&final_paths)); assert!( - links.placeholder_to_target.is_empty(), + links.placeholder_to_source_and_target.is_empty(), "should not generate placeholders when final_paths is provided" ); let beam_links = links.path_links.get(&beam).expect("beam should have refs"); - assert_eq!(beam_links[0].path, "Beams1/001~3"); + assert_eq!( + beam_links[0].path, "./001~3.model.json5", + "should produce relative path (sibling)" + ); } } diff --git a/src/web/api.rs b/src/web/api.rs index 31ceb1ec..db0bf0a7 100644 --- a/src/web/api.rs +++ b/src/web/api.rs @@ -2691,15 +2691,13 @@ impl ApiService { Some(Variant::Ref(target_ref)) => { let attr_name = crate::ref_attribute_name(key); if target_ref.is_none() { - // Null ref: mark attribute for removal remove_attributes.push(attr_name); } else if tree.get_instance(*target_ref).is_some() { - // Valid target: compute path and add as Rojo_Ref_* attribute - let path = crate::ref_target_path_from_tree(tree, *target_ref); + let source_abs = + crate::ref_target_path_from_tree(tree, update.id); + let target_abs = + crate::ref_target_path_from_tree(tree, *target_ref); - // Warn if the path is ambiguous (duplicate-named siblings - // at any ancestor level). The ref will still be written, - // but may resolve to the wrong target on rebuild. if !Self::is_ref_path_unique(tree, *target_ref) { log::warn!( "Ref property '{}' for instance {:?} has an ambiguous \ @@ -2707,24 +2705,29 @@ impl ApiService { resolve to the wrong target on rebuild.", key, update.id, - path + target_abs ); } - ref_attributes.insert(attr_name, serde_json::Value::String(path)); - } else if let Some(path) = added_paths.get(target_ref) { - // Target was added in the same /api/write request. - // It's not in the tree yet (VFS watcher hasn't picked it up), - // but we can compute the path from the AddedInstance data. + let relative = + crate::compute_relative_ref_path(&source_abs, &target_abs); + ref_attributes + .insert(attr_name, serde_json::Value::String(relative)); + } else if let Some(target_abs) = added_paths.get(target_ref) { + let source_abs = + crate::ref_target_path_from_tree(tree, update.id); + let relative = + crate::compute_relative_ref_path(&source_abs, target_abs); log::info!( "Ref property '{}' for instance {:?}: target {:?} is a \ - just-added instance, using pre-computed path '{}'", + just-added instance, computed relative path '{}'", key, update.id, target_ref, - path + relative ); - ref_attributes.insert(attr_name, serde_json::Value::String(path.clone())); + ref_attributes + .insert(attr_name, serde_json::Value::String(relative)); } else { log::warn!( "Cannot persist Ref property '{}' for instance {:?}: \ @@ -2881,6 +2884,7 @@ impl ApiService { // additions/removals (which can miss remaining attrs when one is // removed from a file that has multiple). { + let source_abs = crate::ref_target_path_from_tree(tree, update.id); let mut index = self.ref_path_index.lock().unwrap(); index.remove_all_for_file(&written_meta_path); @@ -2890,7 +2894,12 @@ impl ApiService { for (key, value) in attrs { if key.starts_with(crate::REF_PATH_ATTRIBUTE_PREFIX) { if let Some(path_str) = value.as_str() { - index.add(path_str, &written_meta_path); + let resolved = + crate::resolve_ref_path_to_absolute( + path_str, &source_abs, + ) + .unwrap_or_else(|| path_str.to_string()); + index.add(&resolved, &written_meta_path); } } } diff --git a/tests/tests/serve.rs b/tests/tests/serve.rs index d57f3603..eb97c26a 100644 --- a/tests/tests/serve.rs +++ b/tests/tests/serve.rs @@ -857,7 +857,7 @@ fn ref_path_add_attribute_to_existing_meta() { r#"{ "className": "Model", "attributes": { - "Rojo_Ref_PrimaryPart": "Workspace/MyModel/OtherPart" + "Rojo_Ref_PrimaryPart": "@self/OtherPart" } }"#, ) @@ -917,7 +917,7 @@ fn ref_path_new_file_with_ref_attr() { r#"{ "className": "ObjectValue", "attributes": { - "Rojo_Ref_Value": "Workspace/MyModel/Target" + "Rojo_Ref_Value": "./Target" } }"#, ) @@ -946,7 +946,7 @@ fn ref_path_nonexistent_target_no_crash() { r#"{ "className": "Model", "attributes": { - "Rojo_Ref_PrimaryPart": "Workspace/NonExistent/Part" + "Rojo_Ref_PrimaryPart": "./NonExistent/Part" } }"#, ) @@ -976,8 +976,8 @@ fn ref_path_multiple_attributes() { r#"{ "className": "Model", "attributes": { - "Rojo_Ref_PrimaryPart": "Workspace/MyModel/Target", - "Rojo_Ref_CustomRef": "Workspace/MyModel/OtherPart" + "Rojo_Ref_PrimaryPart": "@self/Target", + "Rojo_Ref_CustomRef": "@self/OtherPart" } }"#, ) diff --git a/tests/tests/two_way_sync.rs b/tests/tests/two_way_sync.rs index 14b0a136..ac6114aa 100644 --- a/tests/tests/two_way_sync.rs +++ b/tests/tests/two_way_sync.rs @@ -6147,7 +6147,7 @@ fn ref_set_primary_part() { poll_meta_has_ref_attr( &meta_path, "Rojo_Ref_PrimaryPart", - "Workspace/TestModel/Part1.model.json5", + "@self/Part1.model.json5", ); }); } @@ -6179,7 +6179,7 @@ fn ref_set_object_value() { poll_meta_has_ref_attr( &model_path, "Rojo_Ref_Value", - "Workspace/TestModel/Part1.model.json5", + "./TestModel/Part1.model.json5", ); }); } @@ -6214,7 +6214,7 @@ fn ref_set_primary_part_to_nil() { poll_meta_has_ref_attr( &meta_path, "Rojo_Ref_PrimaryPart", - "Workspace/TestModel/Part1.model.json5", + "@self/Part1.model.json5", ); // Now set PrimaryPart to nil @@ -6297,7 +6297,7 @@ fn ref_change_target() { poll_meta_has_ref_attr( &meta_path, "Rojo_Ref_PrimaryPart", - "Workspace/TestModel/Part1.model.json5", + "@self/Part1.model.json5", ); // Change PrimaryPart to Part2 @@ -6318,7 +6318,7 @@ fn ref_change_target() { poll_meta_has_ref_attr( &meta_path, "Rojo_Ref_PrimaryPart", - "Workspace/TestModel/Part2.model.json5", + "@self/Part2.model.json5", ); }); } @@ -6348,7 +6348,7 @@ fn ref_set_nil_then_set_again() { poll_meta_has_ref_attr( &meta_path, "Rojo_Ref_PrimaryPart", - "Workspace/TestModel/Part1.model.json5", + "@self/Part1.model.json5", ); let mut props2 = UstrMap::default(); @@ -6383,7 +6383,7 @@ fn ref_set_nil_then_set_again() { poll_meta_has_ref_attr( &meta_path, "Rojo_Ref_PrimaryPart", - "Workspace/TestModel/Part1.model.json5", + "@self/Part1.model.json5", ); }); } @@ -6426,7 +6426,7 @@ fn ref_with_name_change() { assert_meta_has_ref_attr( &new_meta, "Rojo_Ref_PrimaryPart", - "Workspace/RenamedModel/Part1.model.json5", + "@self/Part1.model.json5", ); }); } @@ -6457,7 +6457,7 @@ fn ref_only_change_creates_meta() { poll_meta_has_ref_attr( &meta_path, "Rojo_Ref_PrimaryPart", - "Workspace/TestModel/Part1.model.json5", + "@self/Part1.model.json5", ); }); } @@ -6494,7 +6494,7 @@ fn ref_on_model_json5_instance() { poll_meta_has_ref_attr( &model_path, "Rojo_Ref_Value", - "Workspace/TestModel/Part1.model.json5", + "./TestModel/Part1.model.json5", ); }); } @@ -6526,7 +6526,7 @@ fn ref_existing_meta_preserved() { poll_meta_has_ref_attr( &meta_path, "Rojo_Ref_PrimaryPart", - "Workspace/TestModel/Part1.model.json5", + "@self/Part1.model.json5", ); // Verify existing content wasn't clobbered @@ -6599,7 +6599,7 @@ fn ref_mixed_valid_and_invalid() { poll_meta_has_ref_attr( &meta_path, "Rojo_Ref_PrimaryPart", - "Workspace/TestModel/Part1.model.json5", + "@self/Part1.model.json5", ); }); } @@ -6642,7 +6642,7 @@ fn ref_stale_path_after_target_rename() { poll_meta_has_ref_attr( &meta_path, "Rojo_Ref_PrimaryPart", - "Workspace/TestModel/Part1.model.json5", + "@self/Part1.model.json5", ); // Step 2: Rename Part1 to RenamedPart @@ -6665,7 +6665,7 @@ fn ref_stale_path_after_target_rename() { assert_meta_has_ref_attr( &meta_path, "Rojo_Ref_PrimaryPart", - "Workspace/TestModel/RenamedPart.model.json5", + "@self/RenamedPart.model.json5", ); }); } @@ -6699,7 +6699,7 @@ fn ref_stale_path_after_parent_rename() { poll_meta_has_ref_attr( &meta_path, "Rojo_Ref_PrimaryPart", - "Workspace/TestModel/Part1.model.json5", + "@self/Part1.model.json5", ); // Step 2: Rename the MODEL (parent) from TestModel to RenamedModel @@ -6728,7 +6728,7 @@ fn ref_stale_path_after_parent_rename() { assert_meta_has_ref_attr( &new_meta_path, "Rojo_Ref_PrimaryPart", - "Workspace/RenamedModel/Part1.model.json5", + "@self/Part1.model.json5", ); }); } @@ -6792,7 +6792,7 @@ fn ref_to_instance_added_in_same_request() { assert_meta_has_ref_attr( &meta_path, "Rojo_Ref_PrimaryPart", - "Workspace/TestModel/NewPart.model.json5", + "@self/NewPart.model.json5", ); // Verify the tree also has a resolved PrimaryPart Ref (not dangling). @@ -6862,7 +6862,7 @@ fn ref_ambiguous_path_no_crash() { poll_meta_has_ref_attr( &meta_path, "Rojo_Ref_PrimaryPart", - "Workspace/Target.model.json5", + "./Target.model.json5", ); }); } @@ -6960,7 +6960,7 @@ fn ref_startup_index_rename_updates_preexisting_attr() { assert_meta_has_ref_attr( &meta_path, "Rojo_Ref_PrimaryPart", - "Workspace/TestModel/Part1.model.json5", + "@self/Part1.model.json5", ); // Rename Part1 to IndexTestPart WITHOUT any prior /api/write for Refs. @@ -6985,7 +6985,7 @@ fn ref_startup_index_rename_updates_preexisting_attr() { assert_meta_has_ref_attr( &meta_path, "Rojo_Ref_PrimaryPart", - "Workspace/TestModel/IndexTestPart.model.json5", + "@self/IndexTestPart.model.json5", ); }); } @@ -7021,13 +7021,13 @@ fn ref_partial_removal_preserves_remaining_index_entry() { poll_meta_has_ref_attr( &meta_path, "Rojo_Ref_PrimaryPart", - "Workspace/TestModel/Part1.model.json5", + "@self/Part1.model.json5", ); // Verify second ref is there too assert_meta_has_ref_attr( &meta_path, "Rojo_Ref_Adornee", - "Workspace/TestModel/Part2.model.json5", + "@self/Part2.model.json5", ); // Step 2: Remove PrimaryPart (set to nil), keep Adornee @@ -7051,7 +7051,7 @@ fn ref_partial_removal_preserves_remaining_index_entry() { assert_meta_has_ref_attr( &meta_path, "Rojo_Ref_Adornee", - "Workspace/TestModel/Part2.model.json5", + "@self/Part2.model.json5", ); // Step 3: Rename Part2. The RefPathIndex should still have the entry @@ -7074,7 +7074,7 @@ fn ref_partial_removal_preserves_remaining_index_entry() { assert_meta_has_ref_attr( &meta_path, "Rojo_Ref_Adornee", - "Workspace/TestModel/RenamedPart2.model.json5", + "@self/RenamedPart2.model.json5", ); }); } @@ -7108,7 +7108,7 @@ fn ref_rename_to_slugified_name_updates_ref_paths() { poll_meta_has_ref_attr( &meta_path, "Rojo_Ref_PrimaryPart", - "Workspace/TestModel/Part1.model.json5", + "@self/Part1.model.json5", ); // Rename Model to a name with forbidden characters. @@ -7139,7 +7139,7 @@ fn ref_rename_to_slugified_name_updates_ref_paths() { assert_meta_has_ref_attr( &new_meta_path, "Rojo_Ref_PrimaryPart", - "Workspace/Slug_Model/Part1.model.json5", + "@self/Part1.model.json5", ); }); } @@ -7187,7 +7187,7 @@ fn ref_on_standalone_script() { poll_meta_has_ref_attr( &meta_path, "Rojo_Ref_PrimaryPart", - "Workspace/TestModel/Part1.model.json5", + "./TestModel/Part1.model.json5", ); // Verify the wrong path does NOT have the attribute @@ -7233,7 +7233,7 @@ fn ref_on_txt_file_instance() { poll_meta_has_ref_attr( &meta_path, "Rojo_Ref_PrimaryPart", - "Workspace/TestModel/Part1.model.json5", + "./TestModel/Part1.model.json5", ); }); } @@ -7299,7 +7299,7 @@ fn ref_same_batch_add_target_includes_extension() { poll_meta_has_ref_attr( &meta_path, "Rojo_Ref_PrimaryPart", - "Workspace/RefTarget.server.luau", + "./RefTarget.server.luau", ); }); } @@ -7353,7 +7353,7 @@ fn ref_same_batch_add_module_target_includes_extension() { poll_meta_has_ref_attr( &meta_path, "Rojo_Ref_PrimaryPart", - "Workspace/ModuleTarget.luau", + "./ModuleTarget.luau", ); }); } @@ -7398,7 +7398,7 @@ fn ref_same_batch_add_folder_target_no_extension() { let meta_path = session .path() .join("src/Workspace/TestModel/init.meta.json5"); - poll_meta_has_ref_attr(&meta_path, "Rojo_Ref_PrimaryPart", "Workspace/FolderTarget"); + poll_meta_has_ref_attr(&meta_path, "Rojo_Ref_PrimaryPart", "./FolderTarget"); }); }