From bd5b376b82b97c67c294f0f6d44bd5a0f4a8fccb Mon Sep 17 00:00:00 2001 From: Ziyi Zhang Date: Wed, 23 Sep 2026 15:41:07 -0400 Subject: [PATCH] fix(localization): enforce the batch-localize confirmation in code --- skills/localization/SKILL.md | 2 + .../resources/L10nBatchProcessor.cs | 74 +++++++++++++++++-- 2 files changed, 68 insertions(+), 8 deletions(-) diff --git a/skills/localization/SKILL.md b/skills/localization/SKILL.md index 2b9add5..3f01c6c 100644 --- a/skills/localization/SKILL.md +++ b/skills/localization/SKILL.md @@ -332,6 +332,8 @@ To localize an entire project efficiently, use a batch processing script that ha Only proceed once the user has confirmed. The batch processor template is in [resources/L10nBatchProcessor.cs](resources/L10nBatchProcessor.cs). +`LocalizeAll` also enforces this itself, so do not remove or bypass these checks when adapting the template: it refuses to run in batch mode, prompts the user to save or discard unsaved scene changes, and shows a blocking Editor dialog that lists the scenes it will rewrite. Tell the user to watch the Editor and click the confirm button; if they cancel, it throws `OperationCanceledException` and writes nothing. Report that as a cancellation, not an error to work around. + It walks **both** `Text` and `TMP_Text`, and `LocalizeAll` **returns the labels it could not match** (as `scene :: object :: "text"`). Print that list. It is the whole point of the return value: a run that wires 20 labels and silently leaves 9 alone looks identical to a complete one otherwise. The diff --git a/skills/localization/resources/L10nBatchProcessor.cs b/skills/localization/resources/L10nBatchProcessor.cs index a6dee9b..26483e7 100644 --- a/skills/localization/resources/L10nBatchProcessor.cs +++ b/skills/localization/resources/L10nBatchProcessor.cs @@ -19,7 +19,10 @@ /// while FindObjectsByType<TMP_Text> found 13, so a Text-only pass reports success having /// localized almost nothing. /// -/// Only call LocalizeAll() after confirming with the user — it modifies and saves every scene. +/// LocalizeAll() modifies and saves every scene under Assets/, so it enforces its own safeguards +/// in code rather than relying on the caller: it refuses to run in batch mode, asks the user to +/// save or discard unsaved scene changes, and shows a blocking confirmation dialog listing the +/// scenes it will rewrite. Nothing is written unless the user clicks the confirm button. /// It also reports what it did NOT convert; see the return value of LocalizeHierarchy. /// public static class L10nBatchProcessor @@ -34,14 +37,69 @@ public static List LocalizeAll(Dictionary mapping, strin // dirty and save assets it must not touch. Measured on a real project: unscoped found 20 // scenes where the project has 1, and all 19 extras were inside read-only packages // (com.unity.addressables test fixtures). - string[] scenes = AssetDatabase.FindAssets("t:Scene", new[] { "Assets" }); - foreach (var guid in scenes) + var scenePaths = AssetDatabase.FindAssets("t:Scene", new[] { "Assets" }) + .Select(guid => AssetDatabase.GUIDToAssetPath(guid)) + .ToList(); + + // The safeguards live here, in the code path that does the writes, so skipping a + // sentence in the documentation cannot skip them. + // + // 1. Batch mode: EditorUtility.DisplayDialog returns true without showing anything when + // the Editor runs headless, which would turn the confirmation below into a no-op. + if (Application.isBatchMode) + { + throw new System.InvalidOperationException( + "L10nBatchProcessor.LocalizeAll rewrites every scene and needs a user to confirm it " + + "in an interactive Editor. It does not run in batch mode."); + } + + // 2. Unsaved work: OpenScene(..., Single) below closes whatever is open. Let the user save + // or discard it first; Cancel aborts the whole run. + if (!EditorSceneManager.SaveCurrentModifiedScenesIfUserWantsTo()) + { + throw new System.OperationCanceledException( + "Localization batch cancelled: the open scenes have unsaved changes."); + } + + // 3. Explicit confirmation of the destructive write, naming what will change. + const int listed = 10; + var preview = string.Join("\n", scenePaths.Take(listed)); + if (scenePaths.Count > listed) + { + preview += $"\n... and {scenePaths.Count - listed} more"; + } + var confirmed = EditorUtility.DisplayDialog( + "Localize all scenes?", + $"This opens {scenePaths.Count} scene(s) under Assets/, attaches LocalizeStringEvent " + + $"components to matching text, and saves each scene. It cannot be undone from here; " + + $"use version control to revert.\n\n{preview}", + $"Localize {scenePaths.Count} scene(s)", + "Cancel"); + if (!confirmed) { - var path = AssetDatabase.GUIDToAssetPath(guid); - var scene = EditorSceneManager.OpenScene(path, OpenSceneMode.Single); - unmatched.AddRange(LocalizeHierarchy(mapping, table, path)); - EditorSceneManager.MarkSceneDirty(scene); - EditorSceneManager.SaveScene(scene); + throw new System.OperationCanceledException("Localization batch cancelled by the user."); + } + + // Restore the user's scene layout afterwards, so the batch doesn't leave them looking at + // whichever scene happened to be processed last. + var originalSetup = EditorSceneManager.GetSceneManagerSetup(); + try + { + foreach (var path in scenePaths) + { + var scene = EditorSceneManager.OpenScene(path, OpenSceneMode.Single); + unmatched.AddRange(LocalizeHierarchy(mapping, table, path)); + EditorSceneManager.MarkSceneDirty(scene); + EditorSceneManager.SaveScene(scene); + } + } + finally + { + // An untitled scene has no path to reopen, so only restore a layout made of saved scenes. + if (originalSetup.Length > 0 && originalSetup.All(s => !string.IsNullOrEmpty(s.path))) + { + EditorSceneManager.RestoreSceneManagerSetup(originalSetup); + } } return unmatched; }