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.
This commit is contained in:
Joe 2026-02-28 07:23:49 -06:00
parent dcd48642be
commit 57433a0158
26 changed files with 274 additions and 63 deletions

View file

@ -355,3 +355,6 @@ there is some process that is making disconnecting after a one-shot sync complet
[init scripts] [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. 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!] 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.

View file

@ -35,6 +35,10 @@ function InstanceMap.new(onInstanceChanged)
-- Callback invoked when a new instance is inserted into the map. -- Callback invoked when a new instance is inserted into the map.
-- Used by ChangeBatcher to resolve pending unresolved Ref properties. -- Used by ChangeBatcher to resolve pending unresolved Ref properties.
onInstanceInserted = nil, 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) return setmetatable(self, InstanceMap)

View file

@ -71,6 +71,9 @@ local function applyPatch(instanceMap, patch)
-- When the plugin pre-inserts instances with temp IDs and the server -- When the plugin pre-inserts instances with temp IDs and the server
-- broadcasts the same instances with real Refs, remap the InstanceMap -- broadcasts the same instances with real Refs, remap the InstanceMap
-- entries rather than creating duplicates via reify. -- entries rather than creating duplicates via reify.
-- 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 local remapProgress = true
while remapProgress do while remapProgress do
remapProgress = false remapProgress = false
@ -82,8 +85,9 @@ local function applyPatch(instanceMap, patch)
for _, child in parentInstance:GetChildren() do for _, child in parentInstance:GetChildren() do
if child.Name == virtualInstance.Name and child.ClassName == virtualInstance.ClassName then if child.Name == virtualInstance.Name and child.ClassName == virtualInstance.ClassName then
local existingId = instanceMap.fromInstances[child] local existingId = instanceMap.fromInstances[child]
if existingId and existingId ~= id then if existingId and existingId ~= id and instanceMap.scriptsOnlyTempIds[existingId] then
Log.trace("Echo remap: {} -> {} for {}", existingId, id, child:GetFullName()) Log.trace("Echo remap: {} -> {} for {}", existingId, id, child:GetFullName())
instanceMap.scriptsOnlyTempIds[existingId] = nil
instanceMap:removeId(existingId) instanceMap:removeId(existingId)
instanceMap:insert(id, child) instanceMap:insert(id, child)
patch.added[id] = nil patch.added[id] = nil
@ -94,6 +98,7 @@ local function applyPatch(instanceMap, patch)
end end
end end
end end
end
for id, virtualInstance in pairs(patch.added) do for id, virtualInstance in pairs(patch.added) do
if instanceMap.fromIds[id] ~= nil then if instanceMap.fromIds[id] ~= nil then

View file

@ -906,18 +906,39 @@ function ServeSession:__connectScriptsOnlyWatchers()
return return
end end
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 for _, instance in self.__instanceMap.fromIds do
if instance.Parent ~= game then if instance.Parent == game then
continue table.insert(services, instance)
end
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 if not self.__twoWaySync then
return return
end end
if self.__status ~= Status.Connected then if self.__status ~= Status.Connected then
return return
end 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 if not descendant:IsA("LuaSourceContainer") then
return return
end end
@ -936,18 +957,26 @@ function ServeSession:__connectScriptsOnlyWatchers()
end end
local mappedParent = current local mappedParent = current
if not mappedParent then local mappedParentId = mappedParent and self.__instanceMap.fromInstances[mappedParent]
if not mappedParentId then
local rootId = self.__instanceMap.fromInstances[game]
if not rootId then
return return
end end
local mappedParentId = self.__instanceMap.fromInstances[mappedParent] table.insert(chain, 1, service)
if not mappedParentId then mappedParentId = rootId
return
end end
local patch = PatchSet.newEmpty() local patch = PatchSet.newEmpty()
local prevParentId = mappedParentId local prevParentId = mappedParentId
for _, intermediate in chain do 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() local tempId = string.gsub(HttpService:GenerateGUID(false), "-", ""):lower()
patch.added[tempId] = { patch.added[tempId] = {
parent = prevParentId, parent = prevParentId,
@ -957,6 +986,7 @@ function ServeSession:__connectScriptsOnlyWatchers()
children = {}, children = {},
} }
self.__instanceMap:insert(tempId, intermediate) self.__instanceMap:insert(tempId, intermediate)
self.__instanceMap.scriptsOnlyTempIds[tempId] = true
prevParentId = tempId prevParentId = tempId
end end
@ -969,11 +999,13 @@ function ServeSession:__connectScriptsOnlyWatchers()
) )
for tempId in patch.added do for tempId in patch.added do
self.__instanceMap:removeId(tempId) self.__instanceMap:removeId(tempId)
self.__instanceMap.scriptsOnlyTempIds[tempId] = nil
end end
return return
end end
patch.added[scriptTempId] = encoded patch.added[scriptTempId] = encoded
self.__instanceMap:insert(scriptTempId, descendant) self.__instanceMap:insert(scriptTempId, descendant)
self.__instanceMap.scriptsOnlyTempIds[scriptTempId] = true
Log.info( Log.info(
"Scripts-only: detected new script {} outside mapped tree, sending to server", "Scripts-only: detected new script {} outside mapped tree, sending to server",

View file

@ -9,6 +9,7 @@ rootInstanceId: id-2
serverFork: atlas serverFork: atlas
serverVersion: "[server-version]" serverVersion: "[server-version]"
sessionId: id-1 sessionId: id-1
syncScriptsOnly: false
syncSourceOnly: true syncSourceOnly: true
visibleServices: visibleServices:
- Lighting - Lighting

View file

@ -9,6 +9,7 @@ rootInstanceId: id-2
serverFork: atlas serverFork: atlas
serverVersion: "[server-version]" serverVersion: "[server-version]"
sessionId: id-1 sessionId: id-1
syncScriptsOnly: false
syncSourceOnly: true syncSourceOnly: true
visibleServices: visibleServices:
- Lighting - Lighting

View file

@ -9,6 +9,7 @@ rootInstanceId: id-2
serverFork: atlas serverFork: atlas
serverVersion: "[server-version]" serverVersion: "[server-version]"
sessionId: id-1 sessionId: id-1
syncScriptsOnly: false
syncSourceOnly: true syncSourceOnly: true
visibleServices: visibleServices:
- Lighting - Lighting

View file

@ -9,6 +9,7 @@ rootInstanceId: id-2
serverFork: atlas serverFork: atlas
serverVersion: "[server-version]" serverVersion: "[server-version]"
sessionId: id-1 sessionId: id-1
syncScriptsOnly: false
syncSourceOnly: true syncSourceOnly: true
visibleServices: visibleServices:
- Lighting - Lighting

View file

@ -9,6 +9,7 @@ rootInstanceId: id-2
serverFork: atlas serverFork: atlas
serverVersion: "[server-version]" serverVersion: "[server-version]"
sessionId: id-1 sessionId: id-1
syncScriptsOnly: false
syncSourceOnly: true syncSourceOnly: true
visibleServices: visibleServices:
- Lighting - Lighting

View file

@ -9,6 +9,7 @@ rootInstanceId: id-2
serverFork: atlas serverFork: atlas
serverVersion: "[server-version]" serverVersion: "[server-version]"
sessionId: id-1 sessionId: id-1
syncScriptsOnly: false
syncSourceOnly: true syncSourceOnly: true
visibleServices: visibleServices:
- Lighting - Lighting

View file

@ -9,6 +9,7 @@ rootInstanceId: id-2
serverFork: atlas serverFork: atlas
serverVersion: "[server-version]" serverVersion: "[server-version]"
sessionId: id-1 sessionId: id-1
syncScriptsOnly: false
syncSourceOnly: true syncSourceOnly: true
visibleServices: visibleServices:
- Lighting - Lighting

View file

@ -9,6 +9,7 @@ rootInstanceId: id-2
serverFork: atlas serverFork: atlas
serverVersion: "[server-version]" serverVersion: "[server-version]"
sessionId: id-1 sessionId: id-1
syncScriptsOnly: false
syncSourceOnly: true syncSourceOnly: true
visibleServices: visibleServices:
- Lighting - Lighting

View file

@ -9,6 +9,7 @@ rootInstanceId: id-2
serverFork: atlas serverFork: atlas
serverVersion: "[server-version]" serverVersion: "[server-version]"
sessionId: id-1 sessionId: id-1
syncScriptsOnly: false
syncSourceOnly: true syncSourceOnly: true
visibleServices: visibleServices:
- Lighting - Lighting

View file

@ -9,6 +9,7 @@ rootInstanceId: id-2
serverFork: atlas serverFork: atlas
serverVersion: "[server-version]" serverVersion: "[server-version]"
sessionId: id-1 sessionId: id-1
syncScriptsOnly: false
syncSourceOnly: true syncSourceOnly: true
visibleServices: visibleServices:
- Lighting - Lighting

View file

@ -9,6 +9,7 @@ rootInstanceId: id-2
serverFork: atlas serverFork: atlas
serverVersion: "[server-version]" serverVersion: "[server-version]"
sessionId: id-1 sessionId: id-1
syncScriptsOnly: false
syncSourceOnly: true syncSourceOnly: true
visibleServices: visibleServices:
- Lighting - Lighting

View file

@ -9,6 +9,7 @@ rootInstanceId: id-2
serverFork: atlas serverFork: atlas
serverVersion: "[server-version]" serverVersion: "[server-version]"
sessionId: id-1 sessionId: id-1
syncScriptsOnly: false
syncSourceOnly: true syncSourceOnly: true
visibleServices: visibleServices:
- Lighting - Lighting

View file

@ -9,6 +9,7 @@ rootInstanceId: id-2
serverFork: atlas serverFork: atlas
serverVersion: "[server-version]" serverVersion: "[server-version]"
sessionId: id-1 sessionId: id-1
syncScriptsOnly: false
syncSourceOnly: true syncSourceOnly: true
visibleServices: visibleServices:
- Lighting - Lighting

View file

@ -9,6 +9,7 @@ rootInstanceId: id-2
serverFork: atlas serverFork: atlas
serverVersion: "[server-version]" serverVersion: "[server-version]"
sessionId: id-1 sessionId: id-1
syncScriptsOnly: false
syncSourceOnly: true syncSourceOnly: true
visibleServices: visibleServices:
- Lighting - Lighting

View file

@ -9,6 +9,7 @@ rootInstanceId: id-2
serverFork: atlas serverFork: atlas
serverVersion: "[server-version]" serverVersion: "[server-version]"
sessionId: id-1 sessionId: id-1
syncScriptsOnly: false
syncSourceOnly: true syncSourceOnly: true
visibleServices: visibleServices:
- Lighting - Lighting

View file

@ -9,6 +9,7 @@ rootInstanceId: id-2
serverFork: atlas serverFork: atlas
serverVersion: "[server-version]" serverVersion: "[server-version]"
sessionId: id-1 sessionId: id-1
syncScriptsOnly: false
syncSourceOnly: true syncSourceOnly: true
visibleServices: visibleServices:
- Lighting - Lighting

View file

@ -9,6 +9,7 @@ rootInstanceId: id-2
serverFork: atlas serverFork: atlas
serverVersion: "[server-version]" serverVersion: "[server-version]"
sessionId: id-1 sessionId: id-1
syncScriptsOnly: false
syncSourceOnly: true syncSourceOnly: true
visibleServices: visibleServices:
- Lighting - Lighting

View file

@ -9,6 +9,7 @@ rootInstanceId: id-2
serverFork: atlas serverFork: atlas
serverVersion: "[server-version]" serverVersion: "[server-version]"
sessionId: id-1 sessionId: id-1
syncScriptsOnly: false
syncSourceOnly: true syncSourceOnly: true
visibleServices: visibleServices:
- Lighting - Lighting

View file

@ -9,6 +9,7 @@ rootInstanceId: id-2
serverFork: atlas serverFork: atlas
serverVersion: "[server-version]" serverVersion: "[server-version]"
sessionId: id-1 sessionId: id-1
syncScriptsOnly: false
syncSourceOnly: true syncSourceOnly: true
visibleServices: visibleServices:
- Lighting - Lighting

View file

@ -9,6 +9,7 @@ rootInstanceId: id-2
serverFork: atlas serverFork: atlas
serverVersion: "[server-version]" serverVersion: "[server-version]"
sessionId: id-1 sessionId: id-1
syncScriptsOnly: false
syncSourceOnly: true syncSourceOnly: true
visibleServices: visibleServices:
- Lighting - Lighting

View file

@ -546,6 +546,7 @@ impl ApiService {
if !request.added.is_empty() { if !request.added.is_empty() {
let tree = self.serve_session.tree(); let tree = self.serve_session.tree();
if self.serve_session.sync_scripts_only() {
// Phase 0: Resolve intermediate container temp IDs to real tree Refs. // Phase 0: Resolve intermediate container temp IDs to real tree Refs.
// The DescendantAdded handler sends containers that already exist in the // The DescendantAdded handler sends containers that already exist in the
// tree as "additions" with temp IDs. Resolve them so child additions // tree as "additions" with temp IDs. Resolve them so child additions
@ -566,11 +567,17 @@ impl ApiService {
if let Some(existing_ref) = if let Some(existing_ref) =
Self::find_child_by_name(&tree, real_parent, &added.name) 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); temp_to_real.insert(*guid, existing_ref);
progress = true; progress = true;
} }
} }
} }
}
if !temp_to_real.is_empty() { if !temp_to_real.is_empty() {
log::debug!( log::debug!(
"Scripts-only temp ID resolution: resolved {} intermediate container(s)", "Scripts-only temp ID resolution: resolved {} intermediate container(s)",
@ -587,6 +594,7 @@ impl ApiService {
updated_in_place.insert(*guid); updated_in_place.insert(*guid);
} }
} }
}
// Pre-compute duplicate sibling info in O(N) for efficient path uniqueness checks // Pre-compute duplicate sibling info in O(N) for efficient path uniqueness checks
// This makes all subsequent path checks O(d) instead of O(d × s) // This makes all subsequent path checks O(d) instead of O(d × s)

View file

@ -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"
);
});
}