From dcd48642be1d09fcfa866bd8b4b9f133f000a01c Mon Sep 17 00:00:00 2001 From: Joe Date: Sat, 28 Feb 2026 06:44:37 -0600 Subject: [PATCH] Refactor error handling and improve code readability in ServeSession and applyPatch - Enhanced error logging in ServeSession by formatting warning messages for better clarity. - Improved readability in applyPatch by restructuring conditional statements for clarity. - Simplified assertions in tests for script pruning to maintain consistency and readability. These changes aim to improve code maintainability and enhance the clarity of error messages. --- .cursor/hooks.json | 9 +++++++++ plugin/src/Reconciler/applyPatch.lua | 7 ++++--- plugin/src/ServeSession.lua | 5 ++++- src/web/api.rs | 11 ++++------- tests/tests/scripts_only_pruning.rs | 20 ++++---------------- 5 files changed, 25 insertions(+), 27 deletions(-) create mode 100644 .cursor/hooks.json diff --git a/.cursor/hooks.json b/.cursor/hooks.json new file mode 100644 index 00000000..723daa94 --- /dev/null +++ b/.cursor/hooks.json @@ -0,0 +1,9 @@ +{ + "version": 1, + "hooks": { + "stop": [ + { "command": "cargo fmt" }, + { "command": "stylua plugin/src" } + ] + } +} diff --git a/plugin/src/Reconciler/applyPatch.lua b/plugin/src/Reconciler/applyPatch.lua index 465fc18c..794fe3a1 100644 --- a/plugin/src/Reconciler/applyPatch.lua +++ b/plugin/src/Reconciler/applyPatch.lua @@ -76,10 +76,11 @@ local function applyPatch(instanceMap, patch) remapProgress = false for id, virtualInstance in pairs(patch.added) do local parentInstance = instanceMap.fromIds[virtualInstance.Parent] - if not parentInstance then continue end + if not parentInstance then + continue + end 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] if existingId and existingId ~= id then Log.trace("Echo remap: {} -> {} for {}", existingId, id, child:GetFullName()) diff --git a/plugin/src/ServeSession.lua b/plugin/src/ServeSession.lua index 7952a8b3..53fd0724 100644 --- a/plugin/src/ServeSession.lua +++ b/plugin/src/ServeSession.lua @@ -963,7 +963,10 @@ function ServeSession:__connectScriptsOnlyWatchers() local scriptTempId = string.gsub(HttpService:GenerateGUID(false), "-", ""):lower() local encoded = encodeInstance(descendant, prevParentId) if not encoded then - Log.warn("Scripts-only: failed to encode new script {}, rolling back temp IDs", descendant:GetFullName()) + Log.warn( + "Scripts-only: failed to encode new script {}, rolling back temp IDs", + descendant:GetFullName() + ) for tempId in patch.added do self.__instanceMap:removeId(tempId) end diff --git a/src/web/api.rs b/src/web/api.rs index eef77cdb..b2f8c10a 100644 --- a/src/web/api.rs +++ b/src/web/api.rs @@ -473,9 +473,7 @@ impl ApiService { let before = request.updated.len(); request.updated.retain(|update| { tree.get_instance(update.id) - .map(|inst| { - crate::snapshot::is_script_class(inst.class_name().as_str()) - }) + .map(|inst| crate::snapshot::is_script_class(inst.class_name().as_str())) .unwrap_or(false) }); let filtered = before - request.updated.len(); @@ -561,8 +559,7 @@ impl ApiService { continue; } let parent = added.parent.unwrap_or(Ref::none()); - let real_parent = - temp_to_real.get(&parent).copied().unwrap_or(parent); + let real_parent = temp_to_real.get(&parent).copied().unwrap_or(parent); if tree.get_instance(real_parent).is_none() { continue; } @@ -3642,10 +3639,10 @@ fn filter_subscribe_message_for_scripts<'a>( // in the tree but weren't part of this patch. Without injection the // plugin receives an orphaned script with an unknown parent. for id in included_ids.iter().copied().collect::>() { - if !msg.added.contains_key(&id) { + if let std::collections::hash_map::Entry::Vacant(e) = msg.added.entry(id) { if let Some(tree_inst) = tree.get_instance(id) { let inst = instance_for_scripts_only(tree_inst, &included_ids); - msg.added.insert(id, inst); + e.insert(inst); } } } diff --git a/tests/tests/scripts_only_pruning.rs b/tests/tests/scripts_only_pruning.rs index 779b5287..1bc87dbd 100644 --- a/tests/tests/scripts_only_pruning.rs +++ b/tests/tests/scripts_only_pruning.rs @@ -18,10 +18,7 @@ fn scripts_only_read_prunes_non_script_subtrees() { .map(|inst| inst.name.as_ref()) .collect(); - assert!( - names.contains(&"TopScript"), - "TopScript should be included" - ); + assert!(names.contains(&"TopScript"), "TopScript should be included"); assert!( names.contains(&"DeepScript"), "DeepScript should be included" @@ -31,18 +28,9 @@ fn scripts_only_read_prunes_non_script_subtrees() { "SomeScript should be included" ); - assert!( - !names.contains(&"PartA"), - "PartA should be pruned" - ); - assert!( - !names.contains(&"PartB"), - "PartB should be pruned" - ); - assert!( - !names.contains(&"SomeModel"), - "SomeModel should be pruned" - ); + assert!(!names.contains(&"PartA"), "PartA should be pruned"); + assert!(!names.contains(&"PartB"), "PartB should be pruned"); + assert!(!names.contains(&"SomeModel"), "SomeModel should be pruned"); assert!( !names.contains(&"PureParts"), "PureParts folder should be pruned (no script descendants)"