From 2a290d6109a8d29a66721b822b797b22363bc832 Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 14 Jul 2026 20:45:56 +0900 Subject: [PATCH 1/2] refactor: extract ToolSettings list controller, row binder, and layout signature Extract Class per Fowler: separate foldout/section shell from ListView ownership, row VisualElement binding, and the pure rebuild-signature algorithm. Pin the signature format with characterization tests so list rebuild decisions stay stable. Co-authored-by: Cursor --- ...ToolSettingsSectionLayoutSignatureTests.cs | 48 ++ ...ettingsSectionLayoutSignatureTests.cs.meta | 11 + .../UIToolkit/Components/ToolListRowData.cs | 71 +++ .../Components/ToolListRowData.cs.meta | 11 + .../Components/ToolSettingsSection.cs | 571 +----------------- .../ToolSettingsSectionLayoutSignature.cs | 47 ++ ...ToolSettingsSectionLayoutSignature.cs.meta | 11 + .../ToolSettingsSectionListViewController.cs | 287 +++++++++ ...lSettingsSectionListViewController.cs.meta | 11 + .../ToolSettingsSectionRowBinder.cs | 227 +++++++ .../ToolSettingsSectionRowBinder.cs.meta | 11 + 11 files changed, 752 insertions(+), 554 deletions(-) create mode 100644 Assets/Tests/Editor/ToolSettingsSectionLayoutSignatureTests.cs create mode 100644 Assets/Tests/Editor/ToolSettingsSectionLayoutSignatureTests.cs.meta create mode 100644 Packages/src/Editor/Presentation/UIToolkit/Components/ToolListRowData.cs create mode 100644 Packages/src/Editor/Presentation/UIToolkit/Components/ToolListRowData.cs.meta create mode 100644 Packages/src/Editor/Presentation/UIToolkit/Components/ToolSettingsSectionLayoutSignature.cs create mode 100644 Packages/src/Editor/Presentation/UIToolkit/Components/ToolSettingsSectionLayoutSignature.cs.meta create mode 100644 Packages/src/Editor/Presentation/UIToolkit/Components/ToolSettingsSectionListViewController.cs create mode 100644 Packages/src/Editor/Presentation/UIToolkit/Components/ToolSettingsSectionListViewController.cs.meta create mode 100644 Packages/src/Editor/Presentation/UIToolkit/Components/ToolSettingsSectionRowBinder.cs create mode 100644 Packages/src/Editor/Presentation/UIToolkit/Components/ToolSettingsSectionRowBinder.cs.meta diff --git a/Assets/Tests/Editor/ToolSettingsSectionLayoutSignatureTests.cs b/Assets/Tests/Editor/ToolSettingsSectionLayoutSignatureTests.cs new file mode 100644 index 0000000000..4008e954e8 --- /dev/null +++ b/Assets/Tests/Editor/ToolSettingsSectionLayoutSignatureTests.cs @@ -0,0 +1,48 @@ +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.Presentation; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Characterization tests for Tool Settings list layout signatures. + /// + public sealed class ToolSettingsSectionLayoutSignatureTests + { + [Test] + public void Create_WhenGroupsHaveTools_IncludesGroupMarkersAndToolFields() + { + // Pins the rebuild signature format used to decide whether the ListView must rebuild. + ToolSettingsSectionData data = new( + showToolSettings: true, + builtInTools: new[] + { + new ToolToggleItem("compile", true, false, "Compile the project") + }, + thirdPartyTools: new[] + { + new ToolToggleItem("vendor.tool", false, true, "Vendor tool") + }, + isRegistryAvailable: true); + + string signature = ToolSettingsSectionLayoutSignature.Create(data); + + Assert.That(signature, Is.EqualTo("B:compile|Compile the project|;T:vendor.tool|Vendor tool|;")); + } + + [Test] + public void Create_WhenGroupsAreEmpty_KeepsEmptyGroupMarkers() + { + // Pins empty-group signatures so collapsed/empty catalogs stay stable across refreshes. + ToolSettingsSectionData data = new( + showToolSettings: true, + builtInTools: System.Array.Empty(), + thirdPartyTools: System.Array.Empty(), + isRegistryAvailable: true); + + string signature = ToolSettingsSectionLayoutSignature.Create(data); + + Assert.That(signature, Is.EqualTo("B:;T:;")); + } + } +} diff --git a/Assets/Tests/Editor/ToolSettingsSectionLayoutSignatureTests.cs.meta b/Assets/Tests/Editor/ToolSettingsSectionLayoutSignatureTests.cs.meta new file mode 100644 index 0000000000..fb0ceae4dc --- /dev/null +++ b/Assets/Tests/Editor/ToolSettingsSectionLayoutSignatureTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: c25e44ffdb1e4c2f928333fdfd7079cd +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Presentation/UIToolkit/Components/ToolListRowData.cs b/Packages/src/Editor/Presentation/UIToolkit/Components/ToolListRowData.cs new file mode 100644 index 0000000000..33a637dea5 --- /dev/null +++ b/Packages/src/Editor/Presentation/UIToolkit/Components/ToolListRowData.cs @@ -0,0 +1,71 @@ +using UnityEngine; + +namespace io.github.hatayama.UnityCliLoop.Presentation +{ + /// + /// Row model for the Tool Settings virtualized list (header, tool, or details). + /// + internal sealed class ToolListRowData + { + public readonly bool IsHeader; + public readonly bool IsDetails; + public readonly string ToolName; + public readonly string Label; + public readonly string SkillDescription; + public bool IsEnabled; + public ToolSettingsSection Owner; + public bool IsTool => !IsHeader && !IsDetails; + + private ToolListRowData( + bool isHeader, + bool isDetails, + string toolName, + string label, + string skillDescription, + bool isEnabled) + { + IsHeader = isHeader; + IsDetails = isDetails; + ToolName = toolName; + Label = label; + SkillDescription = skillDescription; + IsEnabled = isEnabled; + } + + public static ToolListRowData CreateHeader(string label) + { + return new ToolListRowData( + true, + false, + string.Empty, + label, + string.Empty, + true); + } + + public static ToolListRowData CreateTool(ToolToggleItem item) + { + return new ToolListRowData( + false, + false, + item.ToolName, + item.ToolName, + item.SkillDescription, + item.IsEnabled); + } + + public static ToolListRowData CreateDetails(ToolListRowData toolRow) + { + Debug.Assert(toolRow != null, "toolRow must not be null"); + Debug.Assert(toolRow.IsTool, "toolRow must be a tool row"); + + return new ToolListRowData( + false, + true, + toolRow.ToolName, + string.Empty, + toolRow.SkillDescription, + true); + } + } +} diff --git a/Packages/src/Editor/Presentation/UIToolkit/Components/ToolListRowData.cs.meta b/Packages/src/Editor/Presentation/UIToolkit/Components/ToolListRowData.cs.meta new file mode 100644 index 0000000000..57dbe27f8d --- /dev/null +++ b/Packages/src/Editor/Presentation/UIToolkit/Components/ToolListRowData.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 3a032ce2e589496b96f98d44d1acde05 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Presentation/UIToolkit/Components/ToolSettingsSection.cs b/Packages/src/Editor/Presentation/UIToolkit/Components/ToolSettingsSection.cs index 6e5619db27..88a1e109d2 100644 --- a/Packages/src/Editor/Presentation/UIToolkit/Components/ToolSettingsSection.cs +++ b/Packages/src/Editor/Presentation/UIToolkit/Components/ToolSettingsSection.cs @@ -1,11 +1,7 @@ using System; -using System.Collections.Generic; -using System.Text; using UnityEngine; using UnityEngine.UIElements; -using io.github.hatayama.UnityCliLoop.Application; - namespace io.github.hatayama.UnityCliLoop.Presentation { /// @@ -14,23 +10,16 @@ namespace io.github.hatayama.UnityCliLoop.Presentation /// public class ToolSettingsSection { - private const int ToolListRowHeight = 24; - private const int ToolDetailsRowHeight = 132; - private const int InlineToolRowLimit = 40; private const string ToolSettingsInfoText = "Enable or disable tools. Disabled tools are hidden from AI agents."; private readonly Foldout _foldout; private readonly VisualElement _toolSettingsInfoContainer; private readonly VisualElement _toolListContainer; private readonly Label _toolListStatusLabel; - private readonly ListView _toolListView; - private readonly List _toolListRows = new(); - private readonly Dictionary _togglesByToolName = new(); - private string _expandedDetailsToolName = string.Empty; + private readonly ToolSettingsSectionListViewController _listViewController; private bool _isRegistryAvailable; private bool _isUnavailableStateShown; private bool _isLoadingStateShown; - private string _layoutSignature = string.Empty; public event Action OnFoldoutChanged; public event Action OnToolToggled; @@ -45,9 +34,11 @@ public ToolSettingsSection(VisualElement root) SetupToolSettingsInfoText(); _toolListStatusLabel = CreateToolListStatusLabel(); - _toolListView = CreateToolListView(); + _listViewController = new ToolSettingsSectionListViewController( + this, + (toolName, enabled) => OnToolToggled?.Invoke(toolName, enabled)); _toolListContainer.Add(_toolListStatusLabel); - _toolListContainer.Add(_toolListView); + _toolListContainer.Add(_listViewController.ListView); ClearToolList(); SetupBindings(); @@ -85,31 +76,14 @@ public void Update(ToolSettingsSectionData data) public void UpdateSingleToggle(string toolName, bool enabled) { - for (int i = 0; i < _toolListRows.Count; i++) - { - ToolListRowData row = _toolListRows[i]; - if (!row.IsTool || row.ToolName != toolName) - { - continue; - } - - row.IsEnabled = enabled; - break; - } - - if (_togglesByToolName.TryGetValue(toolName, out Toggle toggle)) - { - toggle.SetValueWithoutNotify(enabled); - } - - RefreshToolListView(); + _listViewController.UpdateSingleToggle(toolName, enabled); } private void UpdateDeferredState() { - if (_toolListRows.Count > 0 || _isUnavailableStateShown) + if (_listViewController.HasRows || _isUnavailableStateShown) { - RefreshToolListView(); + _listViewController.RefreshToolListView(); return; } @@ -118,13 +92,9 @@ private void UpdateDeferredState() private void UpdateLoadingState() { - _toolListRows.Clear(); - _togglesByToolName.Clear(); - _layoutSignature = string.Empty; - - HideToolDetails(); + _listViewController.ResetRows(); SetToolListStatus("Loading tools..."); - ViewDataBinder.SetVisible(_toolListView, false); + _listViewController.SetListViewVisible(false); SetToolSettingsInfoVisible(false); _isRegistryAvailable = false; @@ -134,13 +104,9 @@ private void UpdateLoadingState() private void UpdateUnavailableState() { - _toolListRows.Clear(); - _togglesByToolName.Clear(); - _layoutSignature = string.Empty; - - HideToolDetails(); + _listViewController.ResetRows(); SetToolListStatus("Tool registry not yet initialized. Start the server first."); - ViewDataBinder.SetVisible(_toolListView, false); + _listViewController.SetListViewVisible(false); SetToolSettingsInfoVisible(false); _isRegistryAvailable = false; @@ -150,15 +116,9 @@ private void UpdateUnavailableState() private void ClearToolList() { - _toolListRows.Clear(); - _togglesByToolName.Clear(); - _layoutSignature = string.Empty; - - HideToolDetails(); + _listViewController.Clear(); ViewDataBinder.SetVisible(_toolListStatusLabel, false); - ViewDataBinder.SetVisible(_toolListView, false); SetToolSettingsInfoVisible(false); - RefreshToolListView(); _isRegistryAvailable = false; _isUnavailableStateShown = false; @@ -192,113 +152,20 @@ private void SetupToolSettingsInfoText() private void UpdateToolList(ToolSettingsSectionData data) { - string layoutSignature = CreateLayoutSignature(data); - bool shouldRebuild = !_isRegistryAvailable + bool forceRebuild = !_isRegistryAvailable || _isUnavailableStateShown - || _isLoadingStateShown - || _layoutSignature != layoutSignature; + || _isLoadingStateShown; - if (shouldRebuild) - { - Rebuild(data); - _layoutSignature = layoutSignature; - } - else - { - UpdateToggleStates(data.BuiltInTools); - UpdateToggleStates(data.ThirdPartyTools); - } + _listViewController.UpdateToolList(data, forceRebuild); ViewDataBinder.SetVisible(_toolListStatusLabel, false); - ViewDataBinder.SetVisible(_toolListView, true); SetToolSettingsInfoVisible(true); - RefreshToolListView(); _isRegistryAvailable = true; _isUnavailableStateShown = false; _isLoadingStateShown = false; } - private void Rebuild(ToolSettingsSectionData data) - { - _toolListRows.Clear(); - _togglesByToolName.Clear(); - HideToolDetails(); - - if (data.BuiltInTools.Length > 0) - { - _toolListRows.Add(ToolListRowData.CreateHeader("Built-in Tools")); - AddToolRows(data.BuiltInTools); - } - - if (data.ThirdPartyTools.Length > 0) - { - _toolListRows.Add(ToolListRowData.CreateHeader("Third Party Tools")); - AddToolRows(data.ThirdPartyTools); - } - - UpdateToolListHeight(); - RefreshToolListView(); - } - - private void AddToolRows(IReadOnlyList items) - { - for (int i = 0; i < items.Count; i++) - { - ToolToggleItem item = items[i]; - _toolListRows.Add(ToolListRowData.CreateTool(item)); - } - } - - private void UpdateToggleStates(IReadOnlyList items) - { - for (int i = 0; i < items.Count; i++) - { - ToolToggleItem item = items[i]; - UpdateToggleState(item.ToolName, item.IsEnabled); - } - } - - private void UpdateToggleState(string toolName, bool isEnabled) - { - for (int i = 0; i < _toolListRows.Count; i++) - { - ToolListRowData row = _toolListRows[i]; - if (!row.IsTool || row.ToolName != toolName) - { - continue; - } - - row.IsEnabled = isEnabled; - return; - } - } - - private static string CreateLayoutSignature(ToolSettingsSectionData data) - { - StringBuilder builder = new(); - AppendGroupSignature(builder, data.BuiltInTools, "B"); - AppendGroupSignature(builder, data.ThirdPartyTools, "T"); - return builder.ToString(); - } - - private static void AppendGroupSignature(StringBuilder builder, IReadOnlyList items, string group) - { - builder.Append(group); - builder.Append(':'); - - for (int i = 0; i < items.Count; i++) - { - ToolToggleItem item = items[i]; - builder.Append(item.ToolName); - builder.Append('|'); - builder.Append(item.SkillDescription); - builder.Append('|'); - } - - builder.Append(';'); - } - private static Label CreateToolListStatusLabel() { Label label = new(); @@ -307,413 +174,9 @@ private static Label CreateToolListStatusLabel() return label; } - private ListView CreateToolListView() - { - ListView listView = new(); - listView.name = "tool-list-view"; - listView.AddToClassList("unity-cli-loop-tool-list-view"); - listView.virtualizationMethod = CollectionVirtualizationMethod.DynamicHeight; - listView.selectionType = SelectionType.None; - listView.itemsSource = _toolListRows; - listView.makeItem = CreateToolListRowElement; - listView.bindItem = BindToolListRowElement; - listView.unbindItem = UnbindToolListRowElement; - return listView; - } - - private static VisualElement CreateToolListRowElement() - { - VisualElement row = new(); - row.AddToClassList("unity-cli-loop-tool-toggle-row"); - row.AddToClassList("unity-cli-loop-tool-list-row"); - - Toggle toggle = new(); - toggle.name = "tool-list-row-toggle"; - toggle.AddToClassList("unity-cli-loop-tool-toggle-row__toggle"); - toggle.RegisterValueChangedCallback(evt => - { - evt.StopPropagation(); - - if (row.userData is not ToolListRowData item || !item.IsTool) - { - return; - } - - item.Owner?.OnToolToggled?.Invoke(item.ToolName, evt.newValue); - }); - - Label label = new(); - label.name = "tool-list-row-label"; - label.AddToClassList("unity-cli-loop-tool-toggle-row__label"); - label.RegisterCallback(evt => - { - evt.StopPropagation(); - - if (row.userData is not ToolListRowData item || !item.IsTool) - { - return; - } - - Toggle rowToggle = row.Q("tool-list-row-toggle"); - bool newValue = !rowToggle.value; - rowToggle.SetValueWithoutNotify(newValue); - item.Owner?.OnToolToggled?.Invoke(item.ToolName, newValue); - }); - - row.Add(toggle); - row.Add(label); - Button detailsButton = new(); - detailsButton.name = "tool-list-row-details-button"; - detailsButton.text = "Show Details"; - detailsButton.tooltip = "Show tool description"; - detailsButton.AddToClassList("unity-cli-loop-tool-toggle-row__details-button"); - detailsButton.RegisterCallback(evt => - { - evt.StopPropagation(); - - if (row.userData is not ToolListRowData item || !item.IsTool) - { - return; - } - - item.Owner?.ToggleToolDetailsForTool(item.ToolName); - }); - row.Add(detailsButton); - - VisualElement detailsPanel = new(); - detailsPanel.name = "tool-list-row-details"; - detailsPanel.AddToClassList("unity-cli-loop-tool-details-panel"); - - TextField detailsBody = new(); - detailsBody.name = "tool-list-row-details-body"; - detailsBody.isReadOnly = true; - detailsBody.multiline = true; - detailsBody.AddToClassList("unity-cli-loop-tool-details-panel__body"); - detailsPanel.Add(detailsBody); - - row.Add(detailsPanel); - return row; - } - - private void BindToolListRowElement(VisualElement row, int index) - { - Debug.Assert(index >= 0 && index < _toolListRows.Count, "tool list index must be valid"); - - ToolListRowData item = _toolListRows[index]; - item.Owner = this; - row.userData = item; - - Toggle toggle = row.Q("tool-list-row-toggle"); - Button detailsButton = row.Q