-
Notifications
You must be signed in to change notification settings - Fork 340
feat(core,ui): add configurable Bootstrap text and JTabView component #624
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Add a serializable BootstrapText struct that externalizes all ~30 user-facing strings from Bootstrap, enabling per-project localization and branding without modifying framework code. Closes #622 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: JasonXuDeveloper - 傑 <jason@xgamedev.net>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds a configurable BootstrapText struct to make all user-facing Bootstrap strings editable per project (localization/branding), and wires those settings into both the default Core inspector and the enhanced JEngine.UI inspector, with basic editor tests.
Changes:
- Introduces
BootstrapText(serializable) with default English strings and format-string tooltips. - Updates
Bootstrapto usetext.*instead of hardcoded UI/status/dialog strings. - Adds a “Text Settings” inspector section + “Reset to Defaults” button in both inspectors, plus UI tests for section presence.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
UnityProject/Packages/com.jasonxudeveloper.jengine.core/Runtime/BootstrapText.cs |
New serializable struct holding all user-facing Bootstrap strings + defaults. |
UnityProject/Packages/com.jasonxudeveloper.jengine.core/Runtime/Bootstrap.cs |
Replaces hardcoded strings with configurable BootstrapText fields and adds default reset. |
UnityProject/Packages/com.jasonxudeveloper.jengine.core/Editor/CustomEditor/BootstrapEditor.cs |
Adds “Text Settings” group and reset-to-defaults logic to the default inspector. |
UnityProject/Packages/com.jasonxudeveloper.jengine.ui/Editor/Internal/BootstrapEditorUI.cs |
Adds enhanced “Text Settings” section + reset button to the JEngine.UI inspector. |
UnityProject/Packages/com.jasonxudeveloper.jengine.ui/Tests/Editor/Internal/BootstrapEditorUITests.cs |
Adds tests asserting the new Text Settings section/controls exist. |
UnityProject/Packages/com.jasonxudeveloper.jengine.core/Runtime/BootstrapText.cs.meta |
New Unity meta for the added runtime file. |
UnityProject/Packages/com.jasonxudeveloper.jengine.core/Runtime/Bootstrap.cs
Outdated
Show resolved
Hide resolved
UnityProject/Packages/com.jasonxudeveloper.jengine.core/Runtime/Bootstrap.cs
Show resolved
Hide resolved
UnityProject/Packages/com.jasonxudeveloper.jengine.core/Runtime/Bootstrap.cs
Outdated
Show resolved
Hide resolved
UnityProject/Packages/com.jasonxudeveloper.jengine.core/Runtime/Bootstrap.cs
Show resolved
Hide resolved
UnityProject/Packages/com.jasonxudeveloper.jengine.core/Runtime/Bootstrap.cs
Outdated
Show resolved
Hide resolved
UnityProject/Packages/com.jasonxudeveloper.jengine.ui/Editor/Internal/BootstrapEditorUI.cs
Outdated
Show resolved
Hide resolved
...ect/Packages/com.jasonxudeveloper.jengine.ui/Tests/Editor/Internal/BootstrapEditorUITests.cs
Outdated
Show resolved
Hide resolved
UnityProject/Packages/com.jasonxudeveloper.jengine.core/Runtime/Bootstrap.cs
Outdated
Show resolved
Hide resolved
UnityProject/Packages/com.jasonxudeveloper.jengine.ui/Editor/Internal/BootstrapEditorUI.cs
Outdated
Show resolved
Hide resolved
Unity Test Results✅ EditMode: All tests passed Unity Version: 2022.3.55f1 ✅ All tests passed! The PR is ready for review. View workflow run |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #624 +/- ##
==========================================
+ Coverage 93.08% 94.91% +1.83%
==========================================
Files 68 70 +2
Lines 9480 10431 +951
==========================================
+ Hits 8824 9901 +1077
+ Misses 656 530 -126
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
- 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 <noreply@anthropic.com> Signed-off-by: JasonXuDeveloper - 傑 <jason@xgamedev.net>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Auto-approved: Copilot review found no issues and Unity Tests passed (or were skipped for non-code changes).
- Add JTabView tabbed container component with configurable maxTabsPerRow - Refactor BootstrapEditorUI Text Settings into 6 categorized tabs (3x2 grid) - Add 46 new tests across 8 components using reflection for private methods (JButton, JCard, JObjectField, JTextField, JDropdown, JStack, JTabView, BootstrapEditorUI) - Update GitHub instructions to require 93%+ coverage for non-core packages with support for both EditMode and non-interactive PlayMode tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: JasonXuDeveloper - 傑 <jason@xgamedev.net>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: JasonXuDeveloper - 傑 <jason@xgamedev.net>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Auto-approved: Copilot review found no issues and Unity Tests passed (or were skipped for non-code changes).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: JasonXuDeveloper - 傑 <jason@xgamedev.net>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Auto-approved: Copilot review found no issues and Unity Tests passed (or were skipped for non-code changes).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 27 out of 35 changed files in this pull request and generated 5 comments.
UnityProject/Packages/com.jasonxudeveloper.jengine.core/Runtime/BootstrapText.cs
Outdated
Show resolved
Hide resolved
UnityProject/Packages/com.jasonxudeveloper.jengine.ui/Editor/Components/Navigation/JTabView.cs
Show resolved
Hide resolved
UnityProject/Packages/com.jasonxudeveloper.jengine.ui/Editor/Components/Navigation/JTabView.cs
Show resolved
Hide resolved
...ect/Packages/com.jasonxudeveloper.jengine.ui/Tests/Editor/Internal/BootstrapEditorUITests.cs
Show resolved
Hide resolved
UnityProject/Packages/com.jasonxudeveloper.jengine.ui/Editor/Components/Navigation/JTabView.cs
Outdated
Show resolved
Hide resolved
- Add null guard to BootstrapText.SafeFormat for null templates - Make JTabView._maxTabsPerRow readonly - Use evt.currentTarget instead of evt.target in JTabView handlers - Add SafeFormat null template test Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: JasonXuDeveloper - 傑 <jason@xgamedev.net>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Auto-approved: Copilot review found no issues and Unity Tests passed (or were skipped for non-code changes).
The currentTarget setter is internal in UIElements. Use reflection with a null check so tests gracefully skip if the API changes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: JasonXuDeveloper - 傑 <jason@xgamedev.net>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Auto-approved: Copilot review found no issues and Unity Tests passed (or were skipped for non-code changes).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 27 out of 35 changed files in this pull request and generated 7 comments.
| // Constrain tabs per row via percentage width | ||
| if (_maxTabsPerRow > 0) | ||
| { | ||
| // Reduce basis to account for per-tab margins; flexGrow fills remaining space | ||
| var percent = (100f - (_maxTabsPerRow * 2f)) / _maxTabsPerRow; | ||
| tabButton.style.flexBasis = new StyleLength(new Length(percent, LengthUnit.Percent)); | ||
| tabButton.style.flexGrow = 1; | ||
| tabButton.style.flexShrink = 1; | ||
| } |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The flexBasis calculation for _maxTabsPerRow subtracts (_maxTabsPerRow * 2f) from 100f (percent units) to “account for margins”, but the margins are set in pixels (Tokens.Spacing.Xs). This mixes units and will behave inconsistently across inspector widths. Consider using a pure percentage basis (100f / _maxTabsPerRow) and let pixel margins naturally reduce available space (or compute basis without assuming percent-based margins).
| var dialogContentTab = new VisualElement(); | ||
| AddTextField(dialogContentTab, textProperty, nameof(BootstrapText.dialogInitFailed), "Init Failed"); | ||
| AddTextField(dialogContentTab, textProperty, nameof(BootstrapText.dialogDownloadPrompt), "Download Prompt"); | ||
| AddTextField(dialogContentTab, textProperty, nameof(BootstrapText.dialogDownloadProgress), "Download Progress"); | ||
| AddTextField(dialogContentTab, textProperty, nameof(BootstrapText.dialogSceneLoadFailed), "Scene Failed"); | ||
| AddTextField(dialogContentTab, textProperty, nameof(BootstrapText.dialogInitException), "Init Exception"); | ||
| AddTextField(dialogContentTab, textProperty, nameof(BootstrapText.dialogCodeException), "Code Exception"); | ||
| AddTextField(dialogContentTab, textProperty, nameof(BootstrapText.dialogFunctionCallFailed), "Call Failed"); |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The enhanced inspector’s Dialog Content fields don’t surface the placeholder guidance from BootstrapText (e.g., {0}, {1} tooltips on the format-string fields). Without this, it’s easy to enter a localized string with the wrong placeholders. Consider adding tooltips/help text for the format-string fields (at least dialogInitFailed, dialogDownloadPrompt, dialogDownloadProgress, dialogSceneLoadFailed, dialogInitException, dialogFunctionCallFailed).
| <field signature="int $Obfuz$RVA_Value18" newName="$lC" /> | ||
| <field signature="int $Obfuz$RVA_Value19" newName="$LC" /> | ||
| <field signature="int $Obfuz$RVA_Value20" newName="$fD" /> | ||
| <field signature="string $Obfuz$RVA_Value21" newName="$FD" /> | ||
| <field signature="int $Obfuz$RVA_Value21" newName="$Kc" /> | ||
| <field signature="int $Obfuz$RVA_Value22" newName="$Qd" /> | ||
| <field signature="int $Obfuz$RVA_Value23" newName="$gD" /> | ||
| <field signature="float $Obfuz$RVA_Value24" newName="$GD" /> | ||
| <field signature="float $Obfuz$RVA_Value23" newName="$DC" /> | ||
| <field signature="string $Obfuz$RVA_Value24" newName="$HC" /> |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assets/Obfuz/SymbolObfus/symbol-mapping.xml changes appear to be generated obfuscation mapping updates (including new BootstrapText-related signatures). Since this file can affect runtime symbol resolution/debugging, please confirm it’s intended to be updated as part of this PR; otherwise it may be safer to keep generated mapping changes out of the feature change.
| public static string SafeFormat(string template, params object[] args) | ||
| { | ||
| if (string.IsNullOrEmpty(template)) | ||
| { | ||
| return string.Empty; | ||
| } | ||
|
|
||
| try | ||
| { | ||
| return string.Format(template, args); | ||
| } |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SafeFormat null-guards template, but still passes args directly to string.Format. If a caller passes null explicitly for args (e.g., SafeFormat(template, null)), string.Format will throw ArgumentNullException and can break bootstrap error handling. Consider normalizing args to an empty array (or catching ArgumentNullException) so SafeFormat is safe for all inputs.
| private static string GetStatusText(PackageInitializationStatus status, BootstrapText text) | ||
| { | ||
| return status switch | ||
| { | ||
| PackageInitializationStatus.InitializingPackage => "Initializing resource package...", | ||
| PackageInitializationStatus.GettingVersion => "Getting resource package version...", | ||
| PackageInitializationStatus.UpdatingManifest => "Updating resource manifest...", | ||
| PackageInitializationStatus.CheckingUpdate => "Checking resources to download...", | ||
| PackageInitializationStatus.DownloadingResources => "Downloading resources...", | ||
| PackageInitializationStatus.Completed => "Resource package initialization completed", | ||
| PackageInitializationStatus.Failed => "Initialization failed", | ||
| _ => "Unknown status" | ||
| PackageInitializationStatus.InitializingPackage => text.initializingPackage, | ||
| PackageInitializationStatus.GettingVersion => text.gettingVersion, | ||
| PackageInitializationStatus.UpdatingManifest => text.updatingManifest, | ||
| PackageInitializationStatus.CheckingUpdate => text.checkingUpdate, | ||
| PackageInitializationStatus.DownloadingResources => text.downloadingResources, | ||
| PackageInitializationStatus.Completed => text.packageCompleted, | ||
| PackageInitializationStatus.Failed => text.initializationFailed, | ||
| _ => text.unknownPackageStatus | ||
| }; | ||
| } | ||
|
|
||
| private static string GetSceneLoadStatusText(SceneLoadStatus status) | ||
| private static string GetSceneLoadStatusText(SceneLoadStatus status, BootstrapText text) | ||
| { | ||
| return status switch | ||
| { | ||
| SceneLoadStatus.Loading => "Loading scene...", | ||
| SceneLoadStatus.Completed => "Scene loading completed", | ||
| SceneLoadStatus.Failed => "Scene loading failed", | ||
| _ => "Unknown status" | ||
| SceneLoadStatus.Loading => text.sceneLoading, | ||
| SceneLoadStatus.Completed => text.sceneCompleted, | ||
| SceneLoadStatus.Failed => text.sceneFailed, | ||
| _ => text.unknownSceneStatus |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GetStatusText/GetSceneLoadStatusText now take BootstrapText by value. Since BootstrapText is a 33-field struct, this copies many references on each call (status updates can be frequent). Consider passing it by in (or changing BootstrapText to an immutable class) to avoid repeated struct copying in hot paths.
| @@ -1 +1 @@ | |||
| 260202209 No newline at end of file | |||
| 260208645 No newline at end of file | |||
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR includes updates to YooAsset StreamingAssets package versions/catalog/bundles. These look like generated build artifacts and aren’t mentioned in the PR summary. If they aren’t required for the BootstrapText/JTabView feature, consider removing them from this PR to keep the change focused and avoid accidentally shipping a new built-in catalog/version.
| // Click handler using closure-free pattern via userData | ||
| tabButton.userData = index; | ||
| tabButton.RegisterCallback<MouseDownEvent>(OnTabClicked); | ||
|
|
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AddTab stores the tab index in Label.userData. Since userData is public and commonly used by consumers, this can conflict if callers also use userData for their own metadata. Consider keeping the index in your own internal mapping (e.g., parallel list lookup by label) or using a small private component/class to avoid consuming userData.
Summary
[Serializable] struct BootstrapTextwith all 33 user-facing strings as configurable fields (default = current English text)Bootstrap.cswithtext.*field references, enabling per-project localization/brandingJTabViewtabbed container component with configurablemaxTabsPerRow(rounded button-style tabs, 3x2 grid layout)BootstrapEditorUIfrom flat sub-headers into 6 categorized tabs: Package Init, Scene Load, Inline Status, Dialog Titles, Dialog Buttons, Dialog ContentCloses #622
Test plan
initializingto Chinese) displays correctly at runtimedialogDownloadPromptwith{0},{1}) resolve correctly🤖 Generated with Claude Code