fix(core): address code review feedback

JasonXuDeveloper · claude · JasonXuDeveloper · commit 3d4e791548f1 · 2026-02-07T01:40:02.000+11:00
- Add BootstrapText.SafeFormat helper with try/catch fallback for
  malformed user-edited format strings
- Replace all string.Format calls with SafeFormat in Bootstrap.cs
- Use nameof(BootstrapText.*) instead of string literals in editor UI
- Use BindProperty for JTextField binding (proper undo/prefab support)
- Add tests for SafeFormat, Default field validation, exact field count,
  sub-headers, and reset logic

Co-Authored-By: Claude Opus 4.6 &lt;noreply@anthropic.com&gt;
Signed-off-by: JasonXuDeveloper - 傑 &lt;jason@xgamedev.net&gt;
diff --git a/UnityProject/Packages/com.jasonxudeveloper.jengine.core/Runtime/Bootstrap.cs b/UnityProject/Packages/com.jasonxudeveloper.jengine.core/Runtime/Bootstrap.cs
@@ -210,7 +210,7 @@ private async void Initialize()
             }
             catch (Exception e)
             {
-                await Prompt.ShowDialogAsync(text.dialogTitleError, string.Format(text.dialogInitFailed, e.Message), text.buttonOk, null);
+                await Prompt.ShowDialogAsync(text.dialogTitleError, BootstrapText.SafeFormat(text.dialogInitFailed, e.Message), text.buttonOk, null);
                 Application.Quit();
             }
         }
@@ -244,13 +244,13 @@ private async UniTask InitializeGame()
                         OnVersionUpdate = version => versionText.text = $"v{Application.version}.{version}",
                         OnDownloadPrompt = async (count, size) =>
                             await Prompt.ShowDialogAsync(t.dialogTitleNotice,
-                                string.Format(t.dialogDownloadPrompt, count, $"{size / 1024f / 1024f:F2}"),
+                                BootstrapText.SafeFormat(t.dialogDownloadPrompt, count, $"{size / 1024f / 1024f:F2}"),
                                 t.buttonDownload, t.buttonCancel),
                         OnDownloadProgress = data =>
                         {
                             if (updateStatusText != null)
                             {
-                                updateStatusText.text = string.Format(t.dialogDownloadProgress,
+                                updateStatusText.text = BootstrapText.SafeFormat(t.dialogDownloadProgress,
                                     data.CurrentDownloadCount, data.TotalDownloadCount,
                                     $"{data.CurrentDownloadBytes / 1024f / 1024f:F2}",
                                     $"{data.TotalDownloadBytes / 1024f / 1024f:F2}");
@@ -339,7 +339,7 @@ await Prompt.ShowDialogAsync(t.dialogTitleNotice,
                         OnError = async exception =>
                         {
                             await Prompt.ShowDialogAsync(t.dialogTitleError,
-                                string.Format(t.dialogSceneLoadFailed, exception.Message),
+                                BootstrapText.SafeFormat(t.dialogSceneLoadFailed, exception.Message),
                                 t.buttonRetry, null);
                         }
                     };
@@ -353,7 +353,7 @@ await Prompt.ShowDialogAsync(t.dialogTitleError,
                 catch (Exception ex)
                 {
                     Debug.LogError($"Initialization failed with exception: {ex}");
-                    await Prompt.ShowDialogAsync(text.dialogTitleError, string.Format(text.dialogInitException, ex.Message), text.buttonOk, text.buttonCancel);
+                    await Prompt.ShowDialogAsync(text.dialogTitleError, BootstrapText.SafeFormat(text.dialogInitException, ex.Message), text.buttonOk, text.buttonCancel);
                     // Continue the loop to retry
                 }
             }
@@ -404,7 +404,7 @@ private async UniTask LoadHotCode(Assembly hotUpdateAss)
             catch (Exception e)
             {
                 Debug.LogError($"Failed to invoke hot update method {hotUpdateMethodName}: {e}");
-                await Prompt.ShowDialogAsync(text.dialogTitleError, string.Format(text.dialogFunctionCallFailed, e.Message), text.buttonExit, null);
+                await Prompt.ShowDialogAsync(text.dialogTitleError, BootstrapText.SafeFormat(text.dialogFunctionCallFailed, e.Message), text.buttonExit, null);
                 Application.Quit();
             }
         }
diff --git a/UnityProject/Packages/com.jasonxudeveloper.jengine.core/Runtime/BootstrapText.cs b/UnityProject/Packages/com.jasonxudeveloper.jengine.core/Runtime/BootstrapText.cs
@@ -148,5 +148,21 @@ public struct BootstrapText
             dialogCodeException = "Code exception, please contact customer service",
             dialogFunctionCallFailed = "Function call failed: {0}",
         };
+
+        /// <summary>
+        /// Safe wrapper around <see cref="string.Format(string, object[])"/> that falls back
+        /// to the raw template if the user-edited format string is malformed.
+        /// </summary>
+        public static string SafeFormat(string template, params object[] args)
+        {
+            try
+            {
+                return string.Format(template, args);
+            }
+            catch (FormatException)
+            {
+                return template;
+            }
+        }
     }
 }
diff --git a/UnityProject/Packages/com.jasonxudeveloper.jengine.ui/Editor/Internal/BootstrapEditorUI.cs b/UnityProject/Packages/com.jasonxudeveloper.jengine.ui/Editor/Internal/BootstrapEditorUI.cs
@@ -450,54 +450,54 @@ private static VisualElement CreateTextSettingsSection()
 
             // Package Initialization Status
             AddTextSubHeader(section, "Package Initialization Status");
-            AddTextField(section, textProperty, "initializingPackage", "Initializing");
-            AddTextField(section, textProperty, "gettingVersion", "Getting Version");
-            AddTextField(section, textProperty, "updatingManifest", "Updating Manifest");
-            AddTextField(section, textProperty, "checkingUpdate", "Checking Update");
-            AddTextField(section, textProperty, "downloadingResources", "Downloading");
-            AddTextField(section, textProperty, "packageCompleted", "Completed");
-            AddTextField(section, textProperty, "initializationFailed", "Failed");
-            AddTextField(section, textProperty, "unknownPackageStatus", "Unknown Status");
+            AddTextField(section, textProperty, nameof(BootstrapText.initializingPackage), "Initializing");
+            AddTextField(section, textProperty, nameof(BootstrapText.gettingVersion), "Getting Version");
+            AddTextField(section, textProperty, nameof(BootstrapText.updatingManifest), "Updating Manifest");
+            AddTextField(section, textProperty, nameof(BootstrapText.checkingUpdate), "Checking Update");
+            AddTextField(section, textProperty, nameof(BootstrapText.downloadingResources), "Downloading");
+            AddTextField(section, textProperty, nameof(BootstrapText.packageCompleted), "Completed");
+            AddTextField(section, textProperty, nameof(BootstrapText.initializationFailed), "Failed");
+            AddTextField(section, textProperty, nameof(BootstrapText.unknownPackageStatus), "Unknown Status");
 
             // Scene Load Status
             AddTextSubHeader(section, "Scene Load Status");
-            AddTextField(section, textProperty, "sceneLoading", "Loading");
-            AddTextField(section, textProperty, "sceneCompleted", "Completed");
-            AddTextField(section, textProperty, "sceneFailed", "Failed");
-            AddTextField(section, textProperty, "unknownSceneStatus", "Unknown Status");
+            AddTextField(section, textProperty, nameof(BootstrapText.sceneLoading), "Loading");
+            AddTextField(section, textProperty, nameof(BootstrapText.sceneCompleted), "Completed");
+            AddTextField(section, textProperty, nameof(BootstrapText.sceneFailed), "Failed");
+            AddTextField(section, textProperty, nameof(BootstrapText.unknownSceneStatus), "Unknown Status");
 
             // Inline Status
             AddTextSubHeader(section, "Inline Status");
-            AddTextField(section, textProperty, "initializing", "Initializing");
-            AddTextField(section, textProperty, "downloading", "Downloading");
-            AddTextField(section, textProperty, "downloadCompletedLoading", "Download Done");
-            AddTextField(section, textProperty, "loadingCode", "Loading Code");
-            AddTextField(section, textProperty, "decryptingResources", "Decrypting");
-            AddTextField(section, textProperty, "loadingScene", "Loading Scene");
+            AddTextField(section, textProperty, nameof(BootstrapText.initializing), "Initializing");
+            AddTextField(section, textProperty, nameof(BootstrapText.downloading), "Downloading");
+            AddTextField(section, textProperty, nameof(BootstrapText.downloadCompletedLoading), "Download Done");
+            AddTextField(section, textProperty, nameof(BootstrapText.loadingCode), "Loading Code");
+            AddTextField(section, textProperty, nameof(BootstrapText.decryptingResources), "Decrypting");
+            AddTextField(section, textProperty, nameof(BootstrapText.loadingScene), "Loading Scene");
 
             // Dialog Titles
             AddTextSubHeader(section, "Dialog Titles");
-            AddTextField(section, textProperty, "dialogTitleError", "Error Title");
-            AddTextField(section, textProperty, "dialogTitleWarning", "Warning Title");
-            AddTextField(section, textProperty, "dialogTitleNotice", "Notice Title");
+            AddTextField(section, textProperty, nameof(BootstrapText.dialogTitleError), "Error Title");
+            AddTextField(section, textProperty, nameof(BootstrapText.dialogTitleWarning), "Warning Title");
+            AddTextField(section, textProperty, nameof(BootstrapText.dialogTitleNotice), "Notice Title");
 
             // Dialog Buttons
             AddTextSubHeader(section, "Dialog Buttons");
-            AddTextField(section, textProperty, "buttonOk", "OK");
-            AddTextField(section, textProperty, "buttonCancel", "Cancel");
-            AddTextField(section, textProperty, "buttonDownload", "Download");
-            AddTextField(section, textProperty, "buttonRetry", "Retry");
-            AddTextField(section, textProperty, "buttonExit", "Exit");
+            AddTextField(section, textProperty, nameof(BootstrapText.buttonOk), "OK");
+            AddTextField(section, textProperty, nameof(BootstrapText.buttonCancel), "Cancel");
+            AddTextField(section, textProperty, nameof(BootstrapText.buttonDownload), "Download");
+            AddTextField(section, textProperty, nameof(BootstrapText.buttonRetry), "Retry");
+            AddTextField(section, textProperty, nameof(BootstrapText.buttonExit), "Exit");
 
             // Dialog Content
             AddTextSubHeader(section, "Dialog Content (Format Strings)");
-            AddTextField(section, textProperty, "dialogInitFailed", "Init Failed");
-            AddTextField(section, textProperty, "dialogDownloadPrompt", "Download Prompt");
-            AddTextField(section, textProperty, "dialogDownloadProgress", "Download Progress");
-            AddTextField(section, textProperty, "dialogSceneLoadFailed", "Scene Failed");
-            AddTextField(section, textProperty, "dialogInitException", "Init Exception");
-            AddTextField(section, textProperty, "dialogCodeException", "Code Exception");
-            AddTextField(section, textProperty, "dialogFunctionCallFailed", "Call Failed");
+            AddTextField(section, textProperty, nameof(BootstrapText.dialogInitFailed), "Init Failed");
+            AddTextField(section, textProperty, nameof(BootstrapText.dialogDownloadPrompt), "Download Prompt");
+            AddTextField(section, textProperty, nameof(BootstrapText.dialogDownloadProgress), "Download Progress");
+            AddTextField(section, textProperty, nameof(BootstrapText.dialogSceneLoadFailed), "Scene Failed");
+            AddTextField(section, textProperty, nameof(BootstrapText.dialogInitException), "Init Exception");
+            AddTextField(section, textProperty, nameof(BootstrapText.dialogCodeException), "Code Exception");
+            AddTextField(section, textProperty, nameof(BootstrapText.dialogFunctionCallFailed), "Call Failed");
 
             // Reset to Defaults button
             var resetButton = new JButton("Reset to Defaults", () =>
@@ -540,12 +540,8 @@ private static void AddTextField(JSection section, SerializedProperty parentProp
             string fieldName, string label)
         {
             var prop = parentProperty.FindPropertyRelative(fieldName);
-            var textField = new JTextField(prop.stringValue);
-            textField.RegisterValueChangedCallback(evt =>
-            {
-                prop.stringValue = evt.newValue;
-                _serializedObject.ApplyModifiedProperties();
-            });
+            var textField = new JTextField();
+            textField.BindProperty(prop);
             section.Add(new JFormField(label, textField));
         }
 
diff --git a/UnityProject/Packages/com.jasonxudeveloper.jengine.ui/Tests/Editor/Internal/BootstrapEditorUITests.cs b/UnityProject/Packages/com.jasonxudeveloper.jengine.ui/Tests/Editor/Internal/BootstrapEditorUITests.cs
@@ -165,16 +165,39 @@ public void CreateInspector_ContainsTextSettingsSection()
         }
 
         [Test]
-        public void TextSettings_ContainsFormFields()
+        public void TextSettings_ContainsExactFieldCount()
         {
             var root = BootstrapEditorUI.CreateInspector(_serializedObject, _bootstrap);
 
             var section = FindSectionByTitle(root, "Text Settings");
             var formFields = section?.Query<JFormField>().ToList();
 
             Assert.IsNotNull(formFields);
-            // Should have all 30 text fields
-            Assert.GreaterOrEqual(formFields.Count, 30);
+            // Must match BootstrapText public instance field count exactly
+            var expectedCount = typeof(BootstrapText)
+                .GetFields(System.Reflection.BindingFlags.Public | System.Reflection.BindingFlags.Instance)
+                .Length;
+            Assert.AreEqual(expectedCount, formFields.Count);
+        }
+
+        [Test]
+        public void TextSettings_ContainsSubHeaders()
+        {
+            var root = BootstrapEditorUI.CreateInspector(_serializedObject, _bootstrap);
+
+            var section = FindSectionByTitle(root, "Text Settings");
+            var labels = section?.Query<Label>().ToList();
+
+            Assert.IsNotNull(labels);
+            bool foundPackageHeader = false;
+            bool foundDialogHeader = false;
+            foreach (var label in labels)
+            {
+                if (label.text == "Package Initialization Status") foundPackageHeader = true;
+                if (label.text == "Dialog Content (Format Strings)") foundDialogHeader = true;
+            }
+            Assert.IsTrue(foundPackageHeader);
+            Assert.IsTrue(foundDialogHeader);
         }
 
         [Test]
@@ -186,7 +209,46 @@ public void TextSettings_ContainsResetButton()
             var buttons = section?.Query<JButton>().ToList();
 
             Assert.IsNotNull(buttons);
-            Assert.GreaterOrEqual(buttons.Count, 1);
+            Assert.AreEqual(1, buttons.Count);
+        }
+
+        [Test]
+        public void TextSettings_ResetLogic_RestoresDefaults()
+        {
+            // Modify a text value via serialized property
+            var textProp = _serializedObject.FindProperty("text");
+            var initProp = textProp.FindPropertyRelative(nameof(BootstrapText.initializing));
+            initProp.stringValue = "Custom text";
+            _serializedObject.ApplyModifiedProperties();
+
+            // Verify value was changed
+            _serializedObject.Update();
+            textProp = _serializedObject.FindProperty("text");
+            initProp = textProp.FindPropertyRelative(nameof(BootstrapText.initializing));
+            Assert.AreEqual("Custom text", initProp.stringValue);
+
+            // Apply reset logic (same as Reset button callback)
+            var defaults = BootstrapText.Default;
+            var fields = typeof(BootstrapText).GetFields(
+                System.Reflection.BindingFlags.Public | System.Reflection.BindingFlags.Instance);
+            foreach (var field in fields)
+            {
+                var prop = textProp.FindPropertyRelative(field.Name);
+                if (prop != null && prop.propertyType == SerializedPropertyType.String)
+                {
+                    prop.stringValue = (string)field.GetValue(defaults);
+                }
+            }
+            _serializedObject.ApplyModifiedProperties();
+
+            // Verify all values were reset
+            _serializedObject.Update();
+            textProp = _serializedObject.FindProperty("text");
+            initProp = textProp.FindPropertyRelative(nameof(BootstrapText.initializing));
+            Assert.AreEqual(BootstrapText.Default.initializing, initProp.stringValue);
+
+            var dlgProp = textProp.FindPropertyRelative(nameof(BootstrapText.dialogTitleError));
+            Assert.AreEqual(BootstrapText.Default.dialogTitleError, dlgProp.stringValue);
         }
 
         #endregion
@@ -574,6 +636,72 @@ public void CreateInspector_MultipleCalls_EachReturnsValidRoot()
 
         #endregion
 
+        #region BootstrapText SafeFormat Tests
+
+        [Test]
+        public void SafeFormat_ValidTemplate_FormatsCorrectly()
+        {
+            var result = BootstrapText.SafeFormat("Hello {0}, you have {1} items", "World", 5);
+            Assert.AreEqual("Hello World, you have 5 items", result);
+        }
+
+        [Test]
+        public void SafeFormat_MalformedTemplate_ReturnsFallback()
+        {
+            var result = BootstrapText.SafeFormat("Bad format {0} {1} {2}", "only_one_arg");
+            Assert.AreEqual("Bad format {0} {1} {2}", result);
+        }
+
+        [Test]
+        public void SafeFormat_InvalidBraces_ReturnsFallback()
+        {
+            var result = BootstrapText.SafeFormat("Broken {", "arg");
+            Assert.AreEqual("Broken {", result);
+        }
+
+        [Test]
+        public void SafeFormat_NoPlaceholders_ReturnsTemplate()
+        {
+            var result = BootstrapText.SafeFormat("No placeholders here");
+            Assert.AreEqual("No placeholders here", result);
+        }
+
+        [Test]
+        public void SafeFormat_EmptyTemplate_ReturnsEmpty()
+        {
+            var result = BootstrapText.SafeFormat("", "arg");
+            Assert.AreEqual("", result);
+        }
+
+        #endregion
+
+        #region BootstrapText Default Tests
+
+        [Test]
+        public void Default_AllFieldsAreNonNull()
+        {
+            var defaults = BootstrapText.Default;
+            var fields = typeof(BootstrapText).GetFields(
+                System.Reflection.BindingFlags.Public | System.Reflection.BindingFlags.Instance);
+
+            foreach (var field in fields)
+            {
+                var value = (string)field.GetValue(defaults);
+                Assert.IsNotNull(value, $"BootstrapText.Default.{field.Name} is null");
+                Assert.IsNotEmpty(value, $"BootstrapText.Default.{field.Name} is empty");
+            }
+        }
+
+        [Test]
+        public void Default_HasExpectedFieldCount()
+        {
+            var fields = typeof(BootstrapText).GetFields(
+                System.Reflection.BindingFlags.Public | System.Reflection.BindingFlags.Instance);
+            Assert.AreEqual(33, fields.Length);
+        }
+
+        #endregion
+
         #region Helper Methods
 
         private static JSection FindSectionByTitle(VisualElement root, string title)

Original file line number	Diff line number	Diff line change
`@@ -210,7 +210,7 @@ private async void Initialize()`
`210`	`210`	`}`
`211`	`211`	`catch (Exception e)`
`212`	`212`	`{`
`213`		`- await Prompt.ShowDialogAsync(text.dialogTitleError, string.Format(text.dialogInitFailed, e.Message), text.buttonOk, null);`
	`213`	`+ await Prompt.ShowDialogAsync(text.dialogTitleError, BootstrapText.SafeFormat(text.dialogInitFailed, e.Message), text.buttonOk, null);`
`214`	`214`	`Application.Quit();`
`215`	`215`	`}`
`216`	`216`	`}`
`@@ -244,13 +244,13 @@ private async UniTask InitializeGame()`
`244`	`244`	`OnVersionUpdate = version => versionText.text = $"v{Application.version}.{version}",`
`245`	`245`	`OnDownloadPrompt = async (count, size) =>`
`246`	`246`	`await Prompt.ShowDialogAsync(t.dialogTitleNotice,`
`247`		`- string.Format(t.dialogDownloadPrompt, count, $"{size / 1024f / 1024f:F2}"),`
	`247`	`+ BootstrapText.SafeFormat(t.dialogDownloadPrompt, count, $"{size / 1024f / 1024f:F2}"),`
`248`	`248`	`t.buttonDownload, t.buttonCancel),`
`249`	`249`	`OnDownloadProgress = data =>`
`250`	`250`	`{`
`251`	`251`	`if (updateStatusText != null)`
`252`	`252`	`{`
`253`		`- updateStatusText.text = string.Format(t.dialogDownloadProgress,`
	`253`	`+ updateStatusText.text = BootstrapText.SafeFormat(t.dialogDownloadProgress,`
`254`	`254`	`data.CurrentDownloadCount, data.TotalDownloadCount,`
`255`	`255`	`$"{data.CurrentDownloadBytes / 1024f / 1024f:F2}",`
`256`	`256`	`$"{data.TotalDownloadBytes / 1024f / 1024f:F2}");`
`@@ -339,7 +339,7 @@ await Prompt.ShowDialogAsync(t.dialogTitleNotice,`
`339`	`339`	`OnError = async exception =>`
`340`	`340`	`{`
`341`	`341`	`await Prompt.ShowDialogAsync(t.dialogTitleError,`
`342`		`- string.Format(t.dialogSceneLoadFailed, exception.Message),`
	`342`	`+ BootstrapText.SafeFormat(t.dialogSceneLoadFailed, exception.Message),`
`343`	`343`	`t.buttonRetry, null);`
`344`	`344`	`}`
`345`	`345`	`};`
`@@ -353,7 +353,7 @@ await Prompt.ShowDialogAsync(t.dialogTitleError,`
`353`	`353`	`catch (Exception ex)`
`354`	`354`	`{`
`355`	`355`	`Debug.LogError($"Initialization failed with exception: {ex}");`
`356`		`- await Prompt.ShowDialogAsync(text.dialogTitleError, string.Format(text.dialogInitException, ex.Message), text.buttonOk, text.buttonCancel);`
	`356`	`+ await Prompt.ShowDialogAsync(text.dialogTitleError, BootstrapText.SafeFormat(text.dialogInitException, ex.Message), text.buttonOk, text.buttonCancel);`
`357`	`357`	`// Continue the loop to retry`
`358`	`358`	`}`
`359`	`359`	`}`
`@@ -404,7 +404,7 @@ private async UniTask LoadHotCode(Assembly hotUpdateAss)`
`404`	`404`	`catch (Exception e)`
`405`	`405`	`{`
`406`	`406`	`Debug.LogError($"Failed to invoke hot update method {hotUpdateMethodName}: {e}");`
`407`		`- await Prompt.ShowDialogAsync(text.dialogTitleError, string.Format(text.dialogFunctionCallFailed, e.Message), text.buttonExit, null);`
	`407`	`+ await Prompt.ShowDialogAsync(text.dialogTitleError, BootstrapText.SafeFormat(text.dialogFunctionCallFailed, e.Message), text.buttonExit, null);`
`408`	`408`	`Application.Quit();`
`409`	`409`	`}`
`410`	`410`	`}`