From 57433a01589c2300b8e06871d0363488c1b2974e Mon Sep 17 00:00:00 2001 From: Joe Date: Sat, 28 Feb 2026 07:23:49 -0600 Subject: [PATCH] Enhance scripts-only mode handling and improve temp ID resolution - Introduced a new mechanism in ServeSession to manage scripts-only mode, ensuring proper handling of temporary IDs during script additions. - Updated the applyPatch function to remap only scripts-only temp IDs, preventing false positives with legitimate additions. - Modified the API to resolve intermediate container temp IDs to real tree references, enhancing the integrity of parent-child relationships for new scripts. - Added tests to verify the functionality of scripts-only mode and the resolution of temp IDs. These changes aim to optimize script management and improve the overall functionality of the session handling. --- TODO.txt | 3 + plugin/src/InstanceMap.lua | 4 + plugin/src/Reconciler/applyPatch.lua | 41 +++--- plugin/src/ServeSession.lua | 50 +++++-- ...to_end__tests__serve__add_folder_info.snap | 1 + ..._to_end__tests__serve__edit_init_info.snap | 1 + .../end_to_end__tests__serve__empty_info.snap | 1 + ...__tests__serve__empty_json_model_info.snap | 1 + ...end__tests__serve__forced_parent_info.snap | 1 + ...__tests__serve__meshpart_with_id_info.snap | 1 + ...sts__serve__move_folder_of_stuff_info.snap | 1 + ...__serve__no_name_default_project_info.snap | 1 + ...d__tests__serve__no_name_project_info.snap | 1 + ...serve__no_name_top_level_project_info.snap | 1 + ...d__tests__serve__pivot_migration_info.snap | 1 + ...nd__tests__serve__ref_properties_info.snap | 1 + ...rve__ref_properties_patch_update_info.snap | 1 + ...ts__serve__ref_properties_remove_info.snap | 1 + ...o_end__tests__serve__remove_file_info.snap | 1 + ...nd_to_end__tests__serve__scripts_info.snap | 1 + ...d__tests__serve__sync_rule_alone_info.snap | 1 + ..._tests__serve__sync_rule_complex_info.snap | 1 + ...s__serve__sync_rule_no_extension_info.snap | 1 + ...serve__sync_rule_no_name_project_info.snap | 1 + src/web/api.rs | 80 +++++----- tests/tests/scripts_only_pruning.rs | 139 ++++++++++++++++++ 26 files changed, 274 insertions(+), 63 deletions(-) diff --git a/TODO.txt b/TODO.txt index 28bd7278..56538631 100644 --- a/TODO.txt +++ b/TODO.txt @@ -355,3 +355,6 @@ there is some process that is making disconnecting after a one-shot sync complet [init scripts] scripts actually named init or probably even Init cause the sync engine to completely break. it will syncback, but then i cannot even serve the project. this needs to be slugified when the name exactly matches init, ignoring case [for anything, not just scripts!] + +[one shot mode change] +new changes that come in / out during the review menu on the plugin is really disorienting. diff --git a/plugin/src/InstanceMap.lua b/plugin/src/InstanceMap.lua index 40b7d761..a5452810 100644 --- a/plugin/src/InstanceMap.lua +++ b/plugin/src/InstanceMap.lua @@ -35,6 +35,10 @@ function InstanceMap.new(onInstanceChanged) -- Callback invoked when a new instance is inserted into the map. -- Used by ChangeBatcher to resolve pending unresolved Ref properties. onInstanceInserted = nil, + + -- Temp IDs generated by DescendantAdded watchers in scripts-only mode. + -- Used by applyPatch to distinguish echo additions from legitimate ones. + scriptsOnlyTempIds = {}, } return setmetatable(self, InstanceMap) diff --git a/plugin/src/Reconciler/applyPatch.lua b/plugin/src/Reconciler/applyPatch.lua index 794fe3a1..61648f77 100644 --- a/plugin/src/Reconciler/applyPatch.lua +++ b/plugin/src/Reconciler/applyPatch.lua @@ -71,24 +71,29 @@ local function applyPatch(instanceMap, patch) -- When the plugin pre-inserts instances with temp IDs and the server -- broadcasts the same instances with real Refs, remap the InstanceMap -- entries rather than creating duplicates via reify. - local remapProgress = true - while remapProgress do - remapProgress = false - for id, virtualInstance in pairs(patch.added) do - local parentInstance = instanceMap.fromIds[virtualInstance.Parent] - if not parentInstance then - continue - end - for _, child in parentInstance:GetChildren() do - if child.Name == virtualInstance.Name and child.ClassName == virtualInstance.ClassName then - local existingId = instanceMap.fromInstances[child] - if existingId and existingId ~= id then - Log.trace("Echo remap: {} -> {} for {}", existingId, id, child:GetFullName()) - instanceMap:removeId(existingId) - instanceMap:insert(id, child) - patch.added[id] = nil - remapProgress = true - break + -- Only remap IDs explicitly tracked as scripts-only temp IDs to avoid + -- false positives with legitimate same-named additions. + if next(instanceMap.scriptsOnlyTempIds) then + local remapProgress = true + while remapProgress do + remapProgress = false + for id, virtualInstance in pairs(patch.added) do + local parentInstance = instanceMap.fromIds[virtualInstance.Parent] + if not parentInstance then + continue + end + for _, child in parentInstance:GetChildren() do + if child.Name == virtualInstance.Name and child.ClassName == virtualInstance.ClassName then + local existingId = instanceMap.fromInstances[child] + if existingId and existingId ~= id and instanceMap.scriptsOnlyTempIds[existingId] then + Log.trace("Echo remap: {} -> {} for {}", existingId, id, child:GetFullName()) + instanceMap.scriptsOnlyTempIds[existingId] = nil + instanceMap:removeId(existingId) + instanceMap:insert(id, child) + patch.added[id] = nil + remapProgress = true + break + end end end end diff --git a/plugin/src/ServeSession.lua b/plugin/src/ServeSession.lua index 53fd0724..198aaeee 100644 --- a/plugin/src/ServeSession.lua +++ b/plugin/src/ServeSession.lua @@ -906,18 +906,39 @@ function ServeSession:__connectScriptsOnlyWatchers() return end - for _, instance in self.__instanceMap.fromIds do - if instance.Parent ~= game then - continue + local services = {} + if self.__serverInfo and self.__serverInfo.visibleServices then + for _, serviceName in ipairs(self.__serverInfo.visibleServices) do + local ok, service = pcall(game.GetService, game, serviceName) + if ok and service then + table.insert(services, service) + end end + else + for _, instance in self.__instanceMap.fromIds do + if instance.Parent == game then + table.insert(services, instance) + end + end + end - local conn = instance.DescendantAdded:Connect(function(descendant) + for _, service in services do + local conn = service.DescendantAdded:Connect(function(descendant) if not self.__twoWaySync then return end if self.__status ~= Status.Connected then return end + if Settings:get("oneShotSync") then + return + end + if RunService:IsRunning() then + return + end + if self.__changeBatcher:isPaused() then + return + end if not descendant:IsA("LuaSourceContainer") then return end @@ -936,18 +957,26 @@ function ServeSession:__connectScriptsOnlyWatchers() end local mappedParent = current - if not mappedParent then - return - end - local mappedParentId = self.__instanceMap.fromInstances[mappedParent] + local mappedParentId = mappedParent and self.__instanceMap.fromInstances[mappedParent] + if not mappedParentId then - return + local rootId = self.__instanceMap.fromInstances[game] + if not rootId then + return + end + table.insert(chain, 1, service) + mappedParentId = rootId end local patch = PatchSet.newEmpty() local prevParentId = mappedParentId for _, intermediate in chain do + local existingId = self.__instanceMap.fromInstances[intermediate] + if existingId then + prevParentId = existingId + continue + end local tempId = string.gsub(HttpService:GenerateGUID(false), "-", ""):lower() patch.added[tempId] = { parent = prevParentId, @@ -957,6 +986,7 @@ function ServeSession:__connectScriptsOnlyWatchers() children = {}, } self.__instanceMap:insert(tempId, intermediate) + self.__instanceMap.scriptsOnlyTempIds[tempId] = true prevParentId = tempId end @@ -969,11 +999,13 @@ function ServeSession:__connectScriptsOnlyWatchers() ) for tempId in patch.added do self.__instanceMap:removeId(tempId) + self.__instanceMap.scriptsOnlyTempIds[tempId] = nil end return end patch.added[scriptTempId] = encoded self.__instanceMap:insert(scriptTempId, descendant) + self.__instanceMap.scriptsOnlyTempIds[scriptTempId] = true Log.info( "Scripts-only: detected new script {} outside mapped tree, sending to server", diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__add_folder_info.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__add_folder_info.snap index 51c8c263..4e552470 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__add_folder_info.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__add_folder_info.snap @@ -9,6 +9,7 @@ rootInstanceId: id-2 serverFork: atlas serverVersion: "[server-version]" sessionId: id-1 +syncScriptsOnly: false syncSourceOnly: true visibleServices: - Lighting diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__edit_init_info.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__edit_init_info.snap index 5a921e2c..291b7cbb 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__edit_init_info.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__edit_init_info.snap @@ -9,6 +9,7 @@ rootInstanceId: id-2 serverFork: atlas serverVersion: "[server-version]" sessionId: id-1 +syncScriptsOnly: false syncSourceOnly: true visibleServices: - Lighting diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__empty_info.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__empty_info.snap index 55ee4ef5..fb3aaf51 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__empty_info.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__empty_info.snap @@ -9,6 +9,7 @@ rootInstanceId: id-2 serverFork: atlas serverVersion: "[server-version]" sessionId: id-1 +syncScriptsOnly: false syncSourceOnly: true visibleServices: - Lighting diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__empty_json_model_info.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__empty_json_model_info.snap index 996e79b2..49ba901f 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__empty_json_model_info.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__empty_json_model_info.snap @@ -9,6 +9,7 @@ rootInstanceId: id-2 serverFork: atlas serverVersion: "[server-version]" sessionId: id-1 +syncScriptsOnly: false syncSourceOnly: true visibleServices: - Lighting diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__forced_parent_info.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__forced_parent_info.snap index af3f5568..cd845477 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__forced_parent_info.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__forced_parent_info.snap @@ -9,6 +9,7 @@ rootInstanceId: id-2 serverFork: atlas serverVersion: "[server-version]" sessionId: id-1 +syncScriptsOnly: false syncSourceOnly: true visibleServices: - Lighting diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__meshpart_with_id_info.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__meshpart_with_id_info.snap index 9b74771e..50df6ea1 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__meshpart_with_id_info.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__meshpart_with_id_info.snap @@ -9,6 +9,7 @@ rootInstanceId: id-2 serverFork: atlas serverVersion: "[server-version]" sessionId: id-1 +syncScriptsOnly: false syncSourceOnly: true visibleServices: - Lighting diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__move_folder_of_stuff_info.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__move_folder_of_stuff_info.snap index b73a82f0..e72e21bb 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__move_folder_of_stuff_info.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__move_folder_of_stuff_info.snap @@ -9,6 +9,7 @@ rootInstanceId: id-2 serverFork: atlas serverVersion: "[server-version]" sessionId: id-1 +syncScriptsOnly: false syncSourceOnly: true visibleServices: - Lighting diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_default_project_info.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_default_project_info.snap index 180281e1..d6513fcd 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_default_project_info.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_default_project_info.snap @@ -9,6 +9,7 @@ rootInstanceId: id-2 serverFork: atlas serverVersion: "[server-version]" sessionId: id-1 +syncScriptsOnly: false syncSourceOnly: true visibleServices: - Lighting diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_project_info.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_project_info.snap index 6c950d3c..bf0f7be4 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_project_info.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_project_info.snap @@ -9,6 +9,7 @@ rootInstanceId: id-2 serverFork: atlas serverVersion: "[server-version]" sessionId: id-1 +syncScriptsOnly: false syncSourceOnly: true visibleServices: - Lighting diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_top_level_project_info.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_top_level_project_info.snap index 6277e107..e1bf19c6 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_top_level_project_info.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_top_level_project_info.snap @@ -9,6 +9,7 @@ rootInstanceId: id-2 serverFork: atlas serverVersion: "[server-version]" sessionId: id-1 +syncScriptsOnly: false syncSourceOnly: true visibleServices: - Lighting diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__pivot_migration_info.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__pivot_migration_info.snap index 22ecb8b9..c9f02a9c 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__pivot_migration_info.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__pivot_migration_info.snap @@ -9,6 +9,7 @@ rootInstanceId: id-2 serverFork: atlas serverVersion: "[server-version]" sessionId: id-1 +syncScriptsOnly: false syncSourceOnly: true visibleServices: - Lighting diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_info.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_info.snap index b103b930..16694999 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_info.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_info.snap @@ -9,6 +9,7 @@ rootInstanceId: id-2 serverFork: atlas serverVersion: "[server-version]" sessionId: id-1 +syncScriptsOnly: false syncSourceOnly: true visibleServices: - Lighting diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_patch_update_info.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_patch_update_info.snap index b103b930..16694999 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_patch_update_info.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_patch_update_info.snap @@ -9,6 +9,7 @@ rootInstanceId: id-2 serverFork: atlas serverVersion: "[server-version]" sessionId: id-1 +syncScriptsOnly: false syncSourceOnly: true visibleServices: - Lighting diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_remove_info.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_remove_info.snap index 08d20d27..572f0ffc 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_remove_info.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_remove_info.snap @@ -9,6 +9,7 @@ rootInstanceId: id-2 serverFork: atlas serverVersion: "[server-version]" sessionId: id-1 +syncScriptsOnly: false syncSourceOnly: true visibleServices: - Lighting diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_file_info.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_file_info.snap index ce635caa..3680063f 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_file_info.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_file_info.snap @@ -9,6 +9,7 @@ rootInstanceId: id-2 serverFork: atlas serverVersion: "[server-version]" sessionId: id-1 +syncScriptsOnly: false syncSourceOnly: true visibleServices: - Lighting diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__scripts_info.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__scripts_info.snap index 55ec17c0..484b57d9 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__scripts_info.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__scripts_info.snap @@ -9,6 +9,7 @@ rootInstanceId: id-2 serverFork: atlas serverVersion: "[server-version]" sessionId: id-1 +syncScriptsOnly: false syncSourceOnly: true visibleServices: - Lighting diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_alone_info.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_alone_info.snap index 3807dd72..8d03ce9e 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_alone_info.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_alone_info.snap @@ -9,6 +9,7 @@ rootInstanceId: id-2 serverFork: atlas serverVersion: "[server-version]" sessionId: id-1 +syncScriptsOnly: false syncSourceOnly: true visibleServices: - Lighting diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_complex_info.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_complex_info.snap index 5835c4c3..9d640eed 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_complex_info.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_complex_info.snap @@ -9,6 +9,7 @@ rootInstanceId: id-2 serverFork: atlas serverVersion: "[server-version]" sessionId: id-1 +syncScriptsOnly: false syncSourceOnly: true visibleServices: - Lighting diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_no_extension_info.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_no_extension_info.snap index 11d3cdbb..09428402 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_no_extension_info.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_no_extension_info.snap @@ -9,6 +9,7 @@ rootInstanceId: id-2 serverFork: atlas serverVersion: "[server-version]" sessionId: id-1 +syncScriptsOnly: false syncSourceOnly: true visibleServices: - Lighting diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_no_name_project_info.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_no_name_project_info.snap index 2ff01d5b..6a823ca4 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_no_name_project_info.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__sync_rule_no_name_project_info.snap @@ -9,6 +9,7 @@ rootInstanceId: id-2 serverFork: atlas serverVersion: "[server-version]" sessionId: id-1 +syncScriptsOnly: false syncSourceOnly: true visibleServices: - Lighting diff --git a/src/web/api.rs b/src/web/api.rs index b2f8c10a..06f63e24 100644 --- a/src/web/api.rs +++ b/src/web/api.rs @@ -546,45 +546,53 @@ impl ApiService { if !request.added.is_empty() { let tree = self.serve_session.tree(); - // Phase 0: Resolve intermediate container temp IDs to real tree Refs. - // The DescendantAdded handler sends containers that already exist in the - // tree as "additions" with temp IDs. Resolve them so child additions - // (the actual new scripts) can find their parents. - let mut temp_to_real: HashMap = HashMap::new(); - let mut progress = true; - while progress { - progress = false; - for (guid, added) in request.added.iter() { - if temp_to_real.contains_key(guid) { - continue; - } - let parent = added.parent.unwrap_or(Ref::none()); - let real_parent = temp_to_real.get(&parent).copied().unwrap_or(parent); - if tree.get_instance(real_parent).is_none() { - continue; - } - if let Some(existing_ref) = - Self::find_child_by_name(&tree, real_parent, &added.name) - { - temp_to_real.insert(*guid, existing_ref); - progress = true; - } - } - } - if !temp_to_real.is_empty() { - log::debug!( - "Scripts-only temp ID resolution: resolved {} intermediate container(s)", - temp_to_real.len() - ); - for added in request.added.values_mut() { - if let Some(parent) = added.parent { - if let Some(&real) = temp_to_real.get(&parent) { - added.parent = Some(real); + if self.serve_session.sync_scripts_only() { + // Phase 0: Resolve intermediate container temp IDs to real tree Refs. + // The DescendantAdded handler sends containers that already exist in the + // tree as "additions" with temp IDs. Resolve them so child additions + // (the actual new scripts) can find their parents. + let mut temp_to_real: HashMap = HashMap::new(); + let mut progress = true; + while progress { + progress = false; + for (guid, added) in request.added.iter() { + if temp_to_real.contains_key(guid) { + continue; + } + let parent = added.parent.unwrap_or(Ref::none()); + let real_parent = temp_to_real.get(&parent).copied().unwrap_or(parent); + if tree.get_instance(real_parent).is_none() { + continue; + } + if let Some(existing_ref) = + Self::find_child_by_name(&tree, real_parent, &added.name) + { + let class_matches = tree + .get_instance(existing_ref) + .map(|inst| inst.class_name().as_str() == added.class_name) + .unwrap_or(false); + if class_matches { + temp_to_real.insert(*guid, existing_ref); + progress = true; + } } } } - for guid in temp_to_real.keys() { - updated_in_place.insert(*guid); + if !temp_to_real.is_empty() { + log::debug!( + "Scripts-only temp ID resolution: resolved {} intermediate container(s)", + temp_to_real.len() + ); + for added in request.added.values_mut() { + if let Some(parent) = added.parent { + if let Some(&real) = temp_to_real.get(&parent) { + added.parent = Some(real); + } + } + } + for guid in temp_to_real.keys() { + updated_in_place.insert(*guid); + } } } diff --git a/tests/tests/scripts_only_pruning.rs b/tests/tests/scripts_only_pruning.rs index 1bc87dbd..0cc668c6 100644 --- a/tests/tests/scripts_only_pruning.rs +++ b/tests/tests/scripts_only_pruning.rs @@ -357,3 +357,142 @@ fn scripts_only_write_resolves_intermediate_temp_ids() { ); }); } + +#[test] +fn normal_mode_addition_works_without_phase0() { + run_serve_test("add_folder", |session, _redactions| { + let info = session.get_api_rojo().unwrap(); + assert!( + !info.sync_scripts_only, + "add_folder fixture should NOT have syncScriptsOnly" + ); + + let root_id = info.root_instance_id; + + let temp_ref = Ref::new(); + let mut props = HashMap::new(); + props.insert( + "Source".to_string(), + Variant::String("print('hello')".to_string()), + ); + let added = AddedInstance { + parent: Some(root_id), + name: "NormalModeScript".to_string(), + class_name: "Script".to_string(), + properties: props, + children: vec![], + }; + let mut added_map = HashMap::new(); + added_map.insert(temp_ref, added); + + let write_request = WriteRequest { + session_id: info.session_id, + removed: vec![], + added: added_map, + updated: vec![], + stage_ids: Vec::new(), + }; + + session + .post_api_write(&write_request) + .expect("Normal mode write should succeed without Phase 0"); + + std::thread::sleep(std::time::Duration::from_millis(500)); + + let read_after = session.get_api_read(root_id).unwrap(); + let names: Vec<&str> = read_after + .instances + .values() + .map(|inst| inst.name.as_ref()) + .collect(); + assert!( + names.contains(&"NormalModeScript"), + "Script should exist after normal-mode addition. Got: {:?}", + names + ); + }); +} + +#[test] +fn scripts_only_phase0_resolves_matching_class() { + run_serve_test("scripts_only_read_pruning", |session, _redactions| { + let info = session.get_api_rojo().unwrap(); + let root_id = info.root_instance_id; + let read_response = session.get_api_read(root_id).unwrap(); + + let sss_id = read_response + .instances + .iter() + .find(|(_, inst)| inst.class_name == "ServerScriptService") + .map(|(id, _)| *id) + .expect("ServerScriptService should exist"); + + // DeepNest exists in the tree as a Folder. Send an intermediate + // container with matching name AND class. Phase 0 should resolve it. + let temp_folder_ref = Ref::new(); + let temp_script_ref = Ref::new(); + + let folder_added = AddedInstance { + parent: Some(sss_id), + name: "DeepNest".to_string(), + class_name: "Folder".to_string(), + properties: HashMap::new(), + children: vec![], + }; + + let mut script_props = HashMap::new(); + script_props.insert( + "Source".to_string(), + Variant::String("return 'matching class test'".to_string()), + ); + let script_added = AddedInstance { + parent: Some(temp_folder_ref), + name: "MatchingClassScript".to_string(), + class_name: "ModuleScript".to_string(), + properties: script_props, + children: vec![], + }; + + let mut added_map = HashMap::new(); + added_map.insert(temp_folder_ref, folder_added); + added_map.insert(temp_script_ref, script_added); + + let write_request = WriteRequest { + session_id: info.session_id, + removed: vec![], + added: added_map, + updated: vec![], + stage_ids: Vec::new(), + }; + + session + .post_api_write(&write_request) + .expect("Write request should succeed"); + + std::thread::sleep(std::time::Duration::from_millis(500)); + + let read_after = session.get_api_read(root_id).unwrap(); + let names: Vec<&str> = read_after + .instances + .values() + .map(|inst| inst.name.as_ref()) + .collect(); + + assert!( + names.contains(&"MatchingClassScript"), + "Script should appear under resolved DeepNest. Got: {:?}", + names + ); + + // Verify there's still only ONE Folder named DeepNest (resolved, not duplicated) + let deep_nest_count = read_after + .instances + .values() + .filter(|inst| inst.name == "DeepNest" && inst.class_name == "Folder") + .count(); + assert_eq!( + deep_nest_count, 1, + "Phase 0 should resolve to existing DeepNest, not create a duplicate" + ); + }); +}