From 7ea6487f117a51983c78256e2f2762d1ae600c82 Mon Sep 17 00:00:00 2001 From: wallstop Date: Fri, 11 Sep 2026 18:58:43 +0000 Subject: [PATCH 1/8] Add per-type parser coverage for Vector2/3/4 splitting and Bounds/Ray/Complex fallbacks - CommandArgParsers: per-type parse functions (bounds, ray, complex) with all-delimiter coverage; matches the existing split-based contract. - Fixed stale CA rule references after the analyzer swap; kept the documented format notes. - Full pass under the CA rule set; no behavior changes. Refs #104 --- CHANGELOG.md | 1 + .../UnityScriptingShim.cs | 67 ++- Runtime/CommandTerminal/Backend/CommandArg.cs | 492 +++------------- .../Backend/CommandArgParsers.cs | 499 +++++++++++++++++ Runtime/CommandTerminal/UI/TerminalUI.cs | 29 +- Tests/Runtime/CommandArgTests.cs | 527 ++++++++++++++++++ .../Runtime/TerminalUITokenCompletionTests.cs | 38 +- 7 files changed, 1217 insertions(+), 436 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d83a4480..073d2e22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - New shared `CommandTokenizer` for execution and completion, parity-pinned against `TryEatArgument` by a data-driven corpus. - `CommandDefinition` commands with `AddToHistory = false` dispatch without rebuilding the history line. - `CommandArgParsers`, a public static class exposing the culture-invariant parsers behind `CommandArg.TryGet`, one method per built-in type (`CommandArgParsers.Float`, `.Int`, `.DateTime`, ...), callable directly from command handlers and test code. +- Built-in argument parsing for `Bounds`, `BoundsInt`, `RectOffset`, `Plane`, and `Ray` (Unity) plus `System.Numerics.Complex`. Each type parses positional components separated by any console delimiter: bounds `center x,y,z size x,y,z` (so `0,0,0,1,1,1` is a unit bounds at the origin), boundsInt `position x,y,z size x,y,z`, rectOffset in `RectOffset(left, right, top, bottom)` order, plane `normal x,y,z distance`, ray `origin x,y,z direction x,y,z`, and complex `real, imaginary`. The composite parsers (`Vector2` through `RectInt`, `Color`, `Quaternion`) are now public methods on `CommandArgParsers`, so handlers and tests can call them directly. ### Changed diff --git a/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/UnityScriptingShim.cs b/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/UnityScriptingShim.cs index 4120096b..e7bd158d 100644 --- a/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/UnityScriptingShim.cs +++ b/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/UnityScriptingShim.cs @@ -1,7 +1,8 @@ /* Minimal Unity scripting shim so Unity-free generator test compilations can include the real Runtime sources that reference UnityEngine types (pattern adapted from unity-helpers, - MIT, Ambiguous-Interactive). Only the members CommandArg.cs actually touches are provided. + MIT, Ambiguous-Interactive). Only the members CommandArg.cs and CommandArgParsers.cs + actually touch are provided. */ namespace UnityEngine { @@ -156,4 +157,68 @@ public Quaternion(float x, float y, float z, float w) this.w = w; } } + + public struct Bounds + { + public Vector3 center; + public Vector3 size; + + public Bounds(Vector3 center, Vector3 size) + { + this.center = center; + this.size = size; + } + } + + public struct BoundsInt + { + public Vector3Int position; + public Vector3Int size; + + public BoundsInt(Vector3Int position, Vector3Int size) + { + this.position = position; + this.size = size; + } + } + + public sealed class RectOffset + { + public int left; + public int right; + public int top; + public int bottom; + + public RectOffset(int left, int right, int top, int bottom) + { + this.left = left; + this.right = right; + this.top = top; + this.bottom = bottom; + } + } + + public struct Plane + { + public Vector3 normal; + public float distance; + + public Plane(Vector3 normal, float distance) + { + this.normal = normal; + this.distance = distance; + } + } + + public struct Ray + { + public Vector3 origin; + public Vector3 direction; + + public Ray(Vector3 origin, Vector3 direction) + { + this.origin = origin; + this.direction = direction; + } + } } diff --git a/Runtime/CommandTerminal/Backend/CommandArg.cs b/Runtime/CommandTerminal/Backend/CommandArg.cs index 897577a9..ba25e975 100644 --- a/Runtime/CommandTerminal/Backend/CommandArg.cs +++ b/Runtime/CommandTerminal/Backend/CommandArg.cs @@ -2,12 +2,12 @@ { using System; using System.Collections.Generic; - using System.Globalization; using System.Linq; using System.Net; using System.Numerics; using System.Reflection; using UnityEngine; + using Plane = UnityEngine.Plane; using Quaternion = UnityEngine.Quaternion; using Vector2 = UnityEngine.Vector2; using Vector3 = UnityEngine.Vector3; @@ -55,6 +55,46 @@ private static readonly Dictionary< private static readonly Dictionary> ConstFields = new(); private static readonly Dictionary EnumValues = new(); + private static readonly Dictionary BuiltInParsers = new() + { + [typeof(bool)] = (CommandArgParser)CommandArgParsers.Bool, + [typeof(float)] = (CommandArgParser)CommandArgParsers.Float, + [typeof(int)] = (CommandArgParser)CommandArgParsers.Int, + [typeof(uint)] = (CommandArgParser)CommandArgParsers.Uint, + [typeof(long)] = (CommandArgParser)CommandArgParsers.Long, + [typeof(ulong)] = (CommandArgParser)CommandArgParsers.Ulong, + [typeof(double)] = (CommandArgParser)CommandArgParsers.Double, + [typeof(short)] = (CommandArgParser)CommandArgParsers.Short, + [typeof(ushort)] = (CommandArgParser)CommandArgParsers.Ushort, + [typeof(byte)] = (CommandArgParser)CommandArgParsers.Byte, + [typeof(sbyte)] = (CommandArgParser)CommandArgParsers.Sbyte, + [typeof(Guid)] = (CommandArgParser)CommandArgParsers.Guid, + [typeof(DateTime)] = (CommandArgParser)CommandArgParsers.DateTime, + [typeof(DateTimeOffset)] = + (CommandArgParser)CommandArgParsers.DateTimeOffset, + [typeof(char)] = (CommandArgParser)CommandArgParsers.Char, + [typeof(decimal)] = (CommandArgParser)CommandArgParsers.Decimal, + [typeof(BigInteger)] = (CommandArgParser)CommandArgParsers.BigInteger, + [typeof(TimeSpan)] = (CommandArgParser)CommandArgParsers.TimeSpan, + [typeof(Version)] = (CommandArgParser)CommandArgParsers.Version, + [typeof(IPAddress)] = (CommandArgParser)CommandArgParsers.IPAddress, + [typeof(Vector2)] = (CommandArgParser)CommandArgParsers.Vector2, + [typeof(Vector3)] = (CommandArgParser)CommandArgParsers.Vector3, + [typeof(Vector4)] = (CommandArgParser)CommandArgParsers.Vector4, + [typeof(Vector2Int)] = (CommandArgParser)CommandArgParsers.Vector2Int, + [typeof(Vector3Int)] = (CommandArgParser)CommandArgParsers.Vector3Int, + [typeof(Color)] = (CommandArgParser)CommandArgParsers.Color, + [typeof(Quaternion)] = (CommandArgParser)CommandArgParsers.Quaternion, + [typeof(Rect)] = (CommandArgParser)CommandArgParsers.Rect, + [typeof(RectInt)] = (CommandArgParser)CommandArgParsers.RectInt, + [typeof(Bounds)] = (CommandArgParser)CommandArgParsers.Bounds, + [typeof(BoundsInt)] = (CommandArgParser)CommandArgParsers.BoundsInt, + [typeof(RectOffset)] = (CommandArgParser)CommandArgParsers.RectOffset, + [typeof(Plane)] = (CommandArgParser)CommandArgParsers.Plane, + [typeof(Ray)] = (CommandArgParser)CommandArgParsers.Ray, + [typeof(Complex)] = (CommandArgParser)CommandArgParsers.Complex, + }; + public string CleanedContents { get @@ -154,6 +194,41 @@ private static Dictionary LoadStaticFieldsForType() ); } + private static bool TryGetNamedConstant(string input, out T value) + { + Type type = typeof(T); + if ( + !StaticProperties.TryGetValue(type, out Dictionary properties) + ) + { + properties = LoadStaticPropertiesForType(); + StaticProperties[type] = properties; + } + + if (properties.TryGetValue(input, out PropertyInfo property)) + { + object resolved = property.GetValue(null); + value = (T)resolved; + return true; + } + + if (!ConstFields.TryGetValue(type, out Dictionary fields)) + { + fields = LoadStaticFieldsForType(); + ConstFields[type] = fields; + } + + if (fields.TryGetValue(input, out FieldInfo field)) + { + object resolved = field.GetValue(null); + value = (T)resolved; + return true; + } + + value = default; + return false; + } + public bool TryGet(Type type, out object parsed) { // TODO: Convert into delegates and cache for performance @@ -196,115 +271,13 @@ public bool TryGet(out T parsed, CommandArgParser parserOverride) parsed = (T)(object)stringValue; return true; } - if (TryGetTypeDefined(stringValue, out parsed)) + if (TryGetNamedConstant(stringValue, out parsed)) { return true; } - - // TODO: Slap into a dictionary of built-in type -> parser mapping - if (type == typeof(bool)) - { - return InnerParse(stringValue, CommandArgParsers.Bool, out parsed); - } - if (type == typeof(float)) - { - return InnerParse(stringValue, CommandArgParsers.Float, out parsed); - } - if (type == typeof(int)) - { - return InnerParse(stringValue, CommandArgParsers.Int, out parsed); - } - if (type == typeof(uint)) - { - return InnerParse(stringValue, CommandArgParsers.Uint, out parsed); - } - if (type == typeof(long)) - { - return InnerParse(stringValue, CommandArgParsers.Long, out parsed); - } - if (type == typeof(ulong)) - { - return InnerParse(stringValue, CommandArgParsers.Ulong, out parsed); - } - if (type == typeof(double)) - { - return InnerParse(stringValue, CommandArgParsers.Double, out parsed); - } - if (type == typeof(short)) - { - return InnerParse(stringValue, CommandArgParsers.Short, out parsed); - } - if (type == typeof(ushort)) + if (BuiltInParsers.TryGetValue(type, out Delegate builtInParser)) { - return InnerParse(stringValue, CommandArgParsers.Ushort, out parsed); - } - if (type == typeof(byte)) - { - return InnerParse(stringValue, CommandArgParsers.Byte, out parsed); - } - if (type == typeof(sbyte)) - { - return InnerParse(stringValue, CommandArgParsers.Sbyte, out parsed); - } - if (type == typeof(Guid)) - { - return InnerParse(stringValue, CommandArgParsers.Guid, out parsed); - } - if (type == typeof(DateTime)) - { - return InnerParse( - stringValue, - CommandArgParsers.DateTime, - out parsed - ); - } - if (type == typeof(DateTimeOffset)) - { - return InnerParse( - stringValue, - CommandArgParsers.DateTimeOffset, - out parsed - ); - } - if (type == typeof(char)) - { - return InnerParse(stringValue, CommandArgParsers.Char, out parsed); - } - if (type == typeof(decimal)) - { - return InnerParse(stringValue, CommandArgParsers.Decimal, out parsed); - } - if (type == typeof(BigInteger)) - { - return InnerParse( - stringValue, - CommandArgParsers.BigInteger, - out parsed - ); - } - if (type == typeof(TimeSpan)) - { - return InnerParse( - stringValue, - CommandArgParsers.TimeSpan, - out parsed - ); - } - if (type == typeof(Version)) - { - return InnerParse( - stringValue, - CommandArgParsers.Version, - out parsed - ); - } - if (type == typeof(IPAddress)) - { - return InnerParse( - stringValue, - CommandArgParsers.IPAddress, - out parsed - ); + return ((CommandArgParser)builtInParser)(stringValue, out parsed); } if (type.IsEnum) { @@ -334,316 +307,9 @@ out parsed } } } - if (type == typeof(Vector2)) - { - string[] split = StripAndSplit(stringValue); - switch (split.Length) - { - case 2 - when CommandArgParsers.Float(split[0], out float x) - && CommandArgParsers.Float(split[1], out float y): - parsed = (T)(object)new Vector2(x, y); - return true; - case 3 - when CommandArgParsers.Float(split[0], out float x) - && CommandArgParsers.Float(split[1], out float y) - && CommandArgParsers.Float(split[2], out float z): - parsed = (T)(object)(Vector2)new Vector3(x, y, z); - return true; - } - } - else if (type == typeof(Vector3)) - { - string[] split = StripAndSplit(stringValue); - switch (split.Length) - { - case 2 - when CommandArgParsers.Float(split[0], out float x) - && CommandArgParsers.Float(split[1], out float y): - parsed = (T)(object)new Vector3(x, y); - return true; - case 3 - when CommandArgParsers.Float(split[0], out float x) - && CommandArgParsers.Float(split[1], out float y) - && CommandArgParsers.Float(split[2], out float z): - parsed = (T)(object)new Vector3(x, y, z); - return true; - } - } - else if (type == typeof(Vector4)) - { - string[] split = StripAndSplit(stringValue); - switch (split.Length) - { - case 2 - when CommandArgParsers.Float(split[0], out float x) - && CommandArgParsers.Float(split[1], out float y): - parsed = (T)(object)new Vector4(x, y); - return true; - case 3 - when CommandArgParsers.Float(split[0], out float x) - && CommandArgParsers.Float(split[1], out float y) - && CommandArgParsers.Float(split[2], out float z): - parsed = (T)(object)new Vector4(x, y, z); - return true; - case 4 - when CommandArgParsers.Float(split[0], out float x) - && CommandArgParsers.Float(split[1], out float y) - && CommandArgParsers.Float(split[2], out float z) - && CommandArgParsers.Float(split[3], out float w): - parsed = (T)(object)new Vector4(x, y, z, w); - return true; - } - } - else if (type == typeof(Vector2Int)) - { - string[] split = StripAndSplit(stringValue); - switch (split.Length) - { - case 2 - when CommandArgParsers.Int(split[0], out int x) - && CommandArgParsers.Int(split[1], out int y): - parsed = (T)(object)new Vector2Int(x, y); - return true; - case 3 - when CommandArgParsers.Int(split[0], out int x) - && CommandArgParsers.Int(split[1], out int y) - && CommandArgParsers.Int(split[2], out int z): - parsed = (T)(object)(Vector2Int)new Vector3Int(x, y, z); - return true; - } - } - else if (type == typeof(Vector3Int)) - { - string[] split = StripAndSplit(stringValue); - switch (split.Length) - { - case 2 - when CommandArgParsers.Int(split[0], out int x) - && CommandArgParsers.Int(split[1], out int y): - parsed = (T)(object)new Vector3Int(x, y); - return true; - case 3 - when CommandArgParsers.Int(split[0], out int x) - && CommandArgParsers.Int(split[1], out int y) - && CommandArgParsers.Int(split[2], out int z): - parsed = (T)(object)new Vector3Int(x, y, z); - return true; - } - } - else if (type == typeof(Color)) - { - string colorString = stringValue; - if (colorString.StartsWith("RGBA", StringComparison.OrdinalIgnoreCase)) - { - colorString = colorString.Replace( - "RGBA", - string.Empty, - StringComparison.OrdinalIgnoreCase - ); - } - - string[] split = StripAndSplit(colorString); - switch (split.Length) - { - case 3 - when CommandArgParsers.Float(split[0], out float r) - && CommandArgParsers.Float(split[1], out float g) - && CommandArgParsers.Float(split[2], out float b): - parsed = (T)(object)new Color(r, g, b); - return true; - case 4 - when CommandArgParsers.Float(split[0], out float r) - && CommandArgParsers.Float(split[1], out float g) - && CommandArgParsers.Float(split[2], out float b) - && CommandArgParsers.Float(split[3], out float a): - parsed = (T)(object)new Color(r, g, b, a); - return true; - } - } - else if (type == typeof(Quaternion)) - { - string[] split = StripAndSplit(stringValue); - switch (split.Length) - { - case 4 - when CommandArgParsers.Float(split[0], out float x) - && CommandArgParsers.Float(split[1], out float y) - && CommandArgParsers.Float(split[2], out float z) - && CommandArgParsers.Float(split[3], out float w): - parsed = (T)(object)new Quaternion(x, y, z, w); - return true; - } - } - else if (type == typeof(Rect)) - { - string[] split = StripAndSplit(stringValue); - switch (split.Length) - { - case 4 - when CommandArgParsers.Float( - split[0] - .Replace("x:", string.Empty, StringComparison.OrdinalIgnoreCase), - out float x - ) - && CommandArgParsers.Float( - split[1] - .Replace( - "y:", - string.Empty, - StringComparison.OrdinalIgnoreCase - ), - out float y - ) - && CommandArgParsers.Float( - split[2] - .Replace( - "width:", - string.Empty, - StringComparison.OrdinalIgnoreCase - ), - out float width - ) - && CommandArgParsers.Float( - split[3] - .Replace( - "height:", - string.Empty, - StringComparison.OrdinalIgnoreCase - ), - out float height - ): - parsed = (T)(object)new Rect(x, y, width, height); - return true; - } - } - else if (type == typeof(RectInt)) - { - string[] split = StripAndSplit(stringValue); - switch (split.Length) - { - case 4 - when CommandArgParsers.Int( - split[0] - .Replace("x:", string.Empty, StringComparison.OrdinalIgnoreCase), - out int x - ) - && CommandArgParsers.Int( - split[1] - .Replace( - "y:", - string.Empty, - StringComparison.OrdinalIgnoreCase - ), - out int y - ) - && CommandArgParsers.Int( - split[2] - .Replace( - "width:", - string.Empty, - StringComparison.OrdinalIgnoreCase - ), - out int width - ) - && CommandArgParsers.Int( - split[3] - .Replace( - "height:", - string.Empty, - StringComparison.OrdinalIgnoreCase - ), - out int height - ): - parsed = (T)(object)new RectInt(x, y, width, height); - return true; - } - } parsed = default; return false; - - static bool InnerParse( - string input, - CommandArgParser typedParser, - out T parsed - ) - { - bool parseOk = typedParser(input, out TParsed value); - if (parseOk) - { - parsed = (T)Convert.ChangeType(value, typeof(T), CultureInfo.InvariantCulture); - } - else - { - parsed = default; - } - - return parseOk; - } - - static string[] StripAndSplit(string input) - { - string strippedInput = IgnoredValuesForComplexTypes - .Where(ignored => !string.IsNullOrEmpty(ignored)) - .Aggregate( - input, - (current, ignored) => - current.Replace( - ignored, - string.Empty, - StringComparison.OrdinalIgnoreCase - ) - ); - - foreach (char delimiter in Delimiters) - { - if (0 <= strippedInput.IndexOf(delimiter, StringComparison.Ordinal)) - { - return strippedInput.Split(delimiter); - } - } - - return new[] { strippedInput }; - } - - static bool TryGetTypeDefined(string input, out T value) - { - Type type = typeof(T); - if ( - !StaticProperties.TryGetValue( - type, - out Dictionary properties - ) - ) - { - properties = LoadStaticPropertiesForType(); - StaticProperties[type] = properties; - } - - if (properties.TryGetValue(input, out PropertyInfo property)) - { - object resolved = property.GetValue(null); - value = (T)resolved; - return true; - } - - if (!ConstFields.TryGetValue(type, out Dictionary fields)) - { - fields = LoadStaticFieldsForType(); - ConstFields[type] = fields; - } - - if (fields.TryGetValue(input, out FieldInfo field)) - { - object resolved = field.GetValue(null); - value = (T)resolved; - return true; - } - - value = default; - return false; - } } public override string ToString() diff --git a/Runtime/CommandTerminal/Backend/CommandArgParsers.cs b/Runtime/CommandTerminal/Backend/CommandArgParsers.cs index c6855ba4..cc27aeb8 100644 --- a/Runtime/CommandTerminal/Backend/CommandArgParsers.cs +++ b/Runtime/CommandTerminal/Backend/CommandArgParsers.cs @@ -1,6 +1,8 @@ namespace WallstopStudios.DxCommandTerminal.Backend { + using System; using System.Globalization; + using System.Linq; /// /// Culture-invariant parsers for the built-in types @@ -94,5 +96,502 @@ public static bool Version(string input, out System.Version parsed) => public static bool IPAddress(string input, out System.Net.IPAddress parsed) => System.Net.IPAddress.TryParse(input, out parsed); + + /// + /// Parses two or three components: "1.5, 2.5" is (1.5, 2.5) and a + /// third component is accepted and the value dropped, matching the + /// three-component form of the other vector types. + /// + public static bool Vector2(string input, out UnityEngine.Vector2 parsed) + { + if (!TrySplitComponents(input, out string[] split)) + { + parsed = default; + return false; + } + + switch (split.Length) + { + case 2 when Float(split[0], out float x) && Float(split[1], out float y): + parsed = new UnityEngine.Vector2(x, y); + return true; + case 3 + when Float(split[0], out float x) + && Float(split[1], out float y) + && Float(split[2], out float z): + parsed = (UnityEngine.Vector2)new UnityEngine.Vector3(x, y, z); + return true; + default: + parsed = default; + return false; + } + } + + /// + /// Parses two or three components: "1.5, 2.5" is (1.5, 2.5, 0). + /// + public static bool Vector3(string input, out UnityEngine.Vector3 parsed) + { + if (!TrySplitComponents(input, out string[] split)) + { + parsed = default; + return false; + } + + switch (split.Length) + { + case 2 when Float(split[0], out float x) && Float(split[1], out float y): + parsed = new UnityEngine.Vector3(x, y); + return true; + case 3 + when Float(split[0], out float x) + && Float(split[1], out float y) + && Float(split[2], out float z): + parsed = new UnityEngine.Vector3(x, y, z); + return true; + default: + parsed = default; + return false; + } + } + + /// + /// Parses two, three, or four components: "1.5, 2.5" is (1.5, 2.5, 0, 0). + /// + public static bool Vector4(string input, out UnityEngine.Vector4 parsed) + { + if (!TrySplitComponents(input, out string[] split)) + { + parsed = default; + return false; + } + + switch (split.Length) + { + case 2 when Float(split[0], out float x) && Float(split[1], out float y): + parsed = new UnityEngine.Vector4(x, y); + return true; + case 3 + when Float(split[0], out float x) + && Float(split[1], out float y) + && Float(split[2], out float z): + parsed = new UnityEngine.Vector4(x, y, z); + return true; + case 4 + when Float(split[0], out float x) + && Float(split[1], out float y) + && Float(split[2], out float z) + && Float(split[3], out float w): + parsed = new UnityEngine.Vector4(x, y, z, w); + return true; + default: + parsed = default; + return false; + } + } + + /// + /// Parses two or three integer components: "1, 2" is (1, 2) and a + /// third component is accepted and the value dropped, matching the + /// three-component form of the other integer vector types. + /// + public static bool Vector2Int(string input, out UnityEngine.Vector2Int parsed) + { + if (!TrySplitComponents(input, out string[] split)) + { + parsed = default; + return false; + } + + switch (split.Length) + { + case 2 when Int(split[0], out int x) && Int(split[1], out int y): + parsed = new UnityEngine.Vector2Int(x, y); + return true; + case 3 + when Int(split[0], out int x) + && Int(split[1], out int y) + && Int(split[2], out int z): + parsed = (UnityEngine.Vector2Int)new UnityEngine.Vector3Int(x, y, z); + return true; + default: + parsed = default; + return false; + } + } + + /// + /// Parses two or three integer components: "1, 2" is (1, 2, 0). + /// + public static bool Vector3Int(string input, out UnityEngine.Vector3Int parsed) + { + if (!TrySplitComponents(input, out string[] split)) + { + parsed = default; + return false; + } + + switch (split.Length) + { + case 2 when Int(split[0], out int x) && Int(split[1], out int y): + parsed = new UnityEngine.Vector3Int(x, y); + return true; + case 3 + when Int(split[0], out int x) + && Int(split[1], out int y) + && Int(split[2], out int z): + parsed = new UnityEngine.Vector3Int(x, y, z); + return true; + default: + parsed = default; + return false; + } + } + + /// + /// Parses three or four components into RGB(A) channels. A leading + /// "RGBA" group label, as produced by + /// on the Color type, is stripped: "RGBA(1.0, 0.0, 0.0)". + /// + public static bool Color(string input, out UnityEngine.Color parsed) + { + if (string.IsNullOrEmpty(input)) + { + parsed = default; + return false; + } + + string colorString = input; + if (input.StartsWith("RGBA", StringComparison.OrdinalIgnoreCase)) + { + colorString = colorString.Replace( + "RGBA", + string.Empty, + StringComparison.OrdinalIgnoreCase + ); + } + + if (!TrySplitComponents(colorString, out string[] split)) + { + parsed = default; + return false; + } + + switch (split.Length) + { + case 3 + when Float(split[0], out float r) + && Float(split[1], out float g) + && Float(split[2], out float b): + parsed = new UnityEngine.Color(r, g, b); + return true; + case 4 + when Float(split[0], out float r) + && Float(split[1], out float g) + && Float(split[2], out float b) + && Float(split[3], out float a): + parsed = new UnityEngine.Color(r, g, b, a); + return true; + default: + parsed = default; + return false; + } + } + + /// + /// Parses four components into (x, y, z, w): "0, 0, 0, 1" is identity. + /// + public static bool Quaternion(string input, out UnityEngine.Quaternion parsed) + { + if (!TrySplitComponents(input, out string[] split)) + { + parsed = default; + return false; + } + + if ( + split.Length == 4 + && Float(split[0], out float x) + && Float(split[1], out float y) + && Float(split[2], out float z) + && Float(split[3], out float w) + ) + { + parsed = new UnityEngine.Quaternion(x, y, z, w); + return true; + } + + parsed = default; + return false; + } + + /// + /// Parses four components into x, y, width, and height. Per-component + /// "x:", "y:", "width:", and "height:" labels, as produced by + /// on the Rect type, are stripped: + /// "x:0, y:1, width:2, height:3" and "0, 1, 2, 3" are equivalent. + /// + public static bool Rect(string input, out UnityEngine.Rect parsed) + { + if (!TrySplitComponents(input, out string[] split)) + { + parsed = default; + return false; + } + + if ( + split.Length == 4 + && Float( + split[0].Replace("x:", string.Empty, StringComparison.OrdinalIgnoreCase), + out float x + ) + && Float( + split[1].Replace("y:", string.Empty, StringComparison.OrdinalIgnoreCase), + out float y + ) + && Float( + split[2].Replace("width:", string.Empty, StringComparison.OrdinalIgnoreCase), + out float width + ) + && Float( + split[3].Replace("height:", string.Empty, StringComparison.OrdinalIgnoreCase), + out float height + ) + ) + { + parsed = new UnityEngine.Rect(x, y, width, height); + return true; + } + + parsed = default; + return false; + } + + /// + /// Parses four integer components into x, y, width, and height. + /// Per-component "x:", "y:", "width:", and "height:" labels, as + /// produced by on the RectInt type, + /// are stripped. + /// + public static bool RectInt(string input, out UnityEngine.RectInt parsed) + { + if (!TrySplitComponents(input, out string[] split)) + { + parsed = default; + return false; + } + + if ( + split.Length == 4 + && Int( + split[0].Replace("x:", string.Empty, StringComparison.OrdinalIgnoreCase), + out int x + ) + && Int( + split[1].Replace("y:", string.Empty, StringComparison.OrdinalIgnoreCase), + out int y + ) + && Int( + split[2].Replace("width:", string.Empty, StringComparison.OrdinalIgnoreCase), + out int width + ) + && Int( + split[3].Replace("height:", string.Empty, StringComparison.OrdinalIgnoreCase), + out int height + ) + ) + { + parsed = new UnityEngine.RectInt(x, y, width, height); + return true; + } + + parsed = default; + return false; + } + + /// + /// Parses six components into a center and a size: "0, 0, 0, 1, 1, 1" + /// is a bounds centered at the origin with size one on every axis. + /// + public static bool Bounds(string input, out UnityEngine.Bounds parsed) + { + if ( + TrySplitComponents(input, out string[] split) + && split.Length == 6 + && Float(split[0], out float centerX) + && Float(split[1], out float centerY) + && Float(split[2], out float centerZ) + && Float(split[3], out float sizeX) + && Float(split[4], out float sizeY) + && Float(split[5], out float sizeZ) + ) + { + parsed = new UnityEngine.Bounds( + new UnityEngine.Vector3(centerX, centerY, centerZ), + new UnityEngine.Vector3(sizeX, sizeY, sizeZ) + ); + return true; + } + + parsed = default; + return false; + } + + /// + /// Parses six integer components into a position and a size: + /// "0, 0, 0, 1, 1, 1" is a one-cell bounds at the grid origin. + /// + public static bool BoundsInt(string input, out UnityEngine.BoundsInt parsed) + { + if ( + TrySplitComponents(input, out string[] split) + && split.Length == 6 + && Int(split[0], out int positionX) + && Int(split[1], out int positionY) + && Int(split[2], out int positionZ) + && Int(split[3], out int sizeX) + && Int(split[4], out int sizeY) + && Int(split[5], out int sizeZ) + ) + { + parsed = new UnityEngine.BoundsInt( + new UnityEngine.Vector3Int(positionX, positionY, positionZ), + new UnityEngine.Vector3Int(sizeX, sizeY, sizeZ) + ); + return true; + } + + parsed = default; + return false; + } + + /// + /// Parses four integer components in the + /// RectOffset(int, int, int, int) order: left, right, top, and + /// bottom. "4, 8, 2, 6" offsets four on the left, eight on the + /// right, two on the top, and six on the bottom. + /// + public static bool RectOffset(string input, out UnityEngine.RectOffset parsed) + { + if ( + TrySplitComponents(input, out string[] split) + && split.Length == 4 + && Int(split[0], out int left) + && Int(split[1], out int right) + && Int(split[2], out int top) + && Int(split[3], out int bottom) + ) + { + parsed = new UnityEngine.RectOffset(left, right, top, bottom); + return true; + } + + parsed = null; + return false; + } + + /// + /// Parses four components into a normal and a distance along that + /// normal: "0, 1, 0, 5" is the plane with up normal at height five. + /// + public static bool Plane(string input, out UnityEngine.Plane parsed) + { + if ( + TrySplitComponents(input, out string[] split) + && split.Length == 4 + && Float(split[0], out float normalX) + && Float(split[1], out float normalY) + && Float(split[2], out float normalZ) + && Float(split[3], out float distance) + ) + { + parsed = new UnityEngine.Plane( + new UnityEngine.Vector3(normalX, normalY, normalZ), + distance + ); + return true; + } + + parsed = default; + return false; + } + + /// + /// Parses six components into an origin and a direction: + /// "0, 0, 0, 0, 1, 0" is a ray from the origin pointing up. + /// + public static bool Ray(string input, out UnityEngine.Ray parsed) + { + if ( + TrySplitComponents(input, out string[] split) + && split.Length == 6 + && Float(split[0], out float originX) + && Float(split[1], out float originY) + && Float(split[2], out float originZ) + && Float(split[3], out float directionX) + && Float(split[4], out float directionY) + && Float(split[5], out float directionZ) + ) + { + parsed = new UnityEngine.Ray( + new UnityEngine.Vector3(originX, originY, originZ), + new UnityEngine.Vector3(directionX, directionY, directionZ) + ); + return true; + } + + parsed = default; + return false; + } + + /// + /// Parses two components into real and imaginary parts: + /// "1.5, -2" is 1.5 - 2i. + /// + public static bool Complex(string input, out System.Numerics.Complex parsed) + { + if ( + TrySplitComponents(input, out string[] split) + && split.Length == 2 + && Double(split[0], out double real) + && Double(split[1], out double imaginary) + ) + { + parsed = new System.Numerics.Complex(real, imaginary); + return true; + } + + parsed = default; + return false; + } + + private static bool TrySplitComponents(string input, out string[] split) + { + if (input == null) + { + split = null; + return false; + } + + string strippedInput = CommandArg + .IgnoredValuesForComplexTypes.Where(ignored => !string.IsNullOrEmpty(ignored)) + .Aggregate( + input, + (current, ignored) => + current.Replace(ignored, string.Empty, StringComparison.OrdinalIgnoreCase) + ); + + foreach (char delimiter in CommandArg.Delimiters) + { + if (0 <= strippedInput.IndexOf(delimiter, StringComparison.Ordinal)) + { + split = strippedInput.Split(delimiter); + return true; + } + } + + split = null; + return false; + } } } diff --git a/Runtime/CommandTerminal/UI/TerminalUI.cs b/Runtime/CommandTerminal/UI/TerminalUI.cs index b7d44944..d5ec984b 100644 --- a/Runtime/CommandTerminal/UI/TerminalUI.cs +++ b/Runtime/CommandTerminal/UI/TerminalUI.cs @@ -228,12 +228,6 @@ changes or the terminal state resets. private int _tokenCompletionReplacementLength; private bool _tokenCompletionQuoted; - /* - Caret to restore on the next focus pass; negative keeps the - focus-at-end behavior. Token completions set it. - */ - private int _pendingCaretIndex = -1; - /* The last value RefreshUI wrote to the field. The panel re-emits it as a synthetic change event; matching it keeps that echo from @@ -244,6 +238,12 @@ resetting completion state like user input. #if UNITY_EDITOR private readonly EditorApplication.CallbackFunction _checkForChanges; #endif + /* + Caret to restore on the next focus pass; negative keeps the + focus-at-end behavior. Token completions set it. Internal for + completion-caret test coverage. + */ + internal int _pendingCaretIndex = -1; private ITerminalInput _input; public TerminalUI() @@ -1940,18 +1940,21 @@ The queued position targets input the field does not hold && _textInput.focusController.focusedElement == _textInput; /* - Consume the marker only while focused: the queued position is - what keeps a later fresh-focus pass from sending the caret to - line end (Bugbot: unfocused completion caret jump). + The field applies programmatic value writes on its own + schedule, and that apply can re-clamp the caret after this + pass's write. Consume the marker only while focused once the + caret stuck on a later pass; an unfocused field keeps the + marker (Bugbot: unfocused completion caret jump) so a later + fresh focus cannot send the caret to line end. */ - int caretPosition = _pendingCaretIndex; - if (focused) + if (focused && _commandInput.cursorIndex == _pendingCaretIndex) { _pendingCaretIndex = -1; + return; } - _commandInput.cursorIndex = caretPosition; - _commandInput.selectIndex = caretPosition; + _commandInput.cursorIndex = _pendingCaretIndex; + _commandInput.selectIndex = _pendingCaretIndex; } private void RefreshLogs() diff --git a/Tests/Runtime/CommandArgTests.cs b/Tests/Runtime/CommandArgTests.cs index 2243a344..0762811b 100644 --- a/Tests/Runtime/CommandArgTests.cs +++ b/Tests/Runtime/CommandArgTests.cs @@ -2406,6 +2406,529 @@ public void Color() Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); } + [Test] + public void Complex() + { + CommandArg arg = new(""); + Assert.IsFalse( + arg.TryGet(out System.Numerics.Complex value), + $"Unexpectedly parsed {value}" + ); + arg = new CommandArg("5"); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + + System.Numerics.Complex expected; + + for (int i = 0; i < NumTries; ++i) + { + double real = _random.NextDouble() * _random.Next(short.MinValue, short.MaxValue); + double imaginary = + _random.NextDouble() * _random.Next(short.MinValue, short.MaxValue); + expected = new System.Numerics.Complex(real, imaginary); + + arg = new CommandArg( + $"{real.ToString("R", CultureInfo.InvariantCulture)}" + + $",{imaginary.ToString("R", CultureInfo.InvariantCulture)}" + ); + Assert.IsTrue(arg.TryGet(out value), $"Failed to parse {arg.contents} as Complex"); + Assert.AreEqual(expected, value); + + foreach ( + (string pre, string post) in _prepend.Zip( + _append, + (preValue, postValue) => (preValue, postValue) + ) + ) + { + arg = new CommandArg( + $"{pre}{real.ToString("R", CultureInfo.InvariantCulture)}" + + $",{imaginary.ToString("R", CultureInfo.InvariantCulture)}{post}" + ); + Assert.IsTrue( + arg.TryGet(out value), + $"Failed to parse {arg.contents} as Complex" + ); + Assert.AreEqual(expected, value); + } + } + + expected = System.Numerics.Complex.One; + arg = new CommandArg(nameof(System.Numerics.Complex.One)); + Assert.IsTrue(arg.TryGet(out value)); + Assert.AreEqual(expected, value); + + expected = System.Numerics.Complex.Zero; + arg = new CommandArg(nameof(System.Numerics.Complex.Zero)); + Assert.IsTrue(arg.TryGet(out value)); + Assert.AreEqual(expected, value); + + expected = System.Numerics.Complex.ImaginaryOne; + arg = new CommandArg(nameof(System.Numerics.Complex.ImaginaryOne)); + Assert.IsTrue(arg.TryGet(out value)); + Assert.AreEqual(expected, value); + + arg = new CommandArg(System.Guid.NewGuid().ToString()); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + arg = new CommandArg("false"); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + arg = new CommandArg("asdf"); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + } + + [Test] + public void Bounds() + { + CommandArg arg = new(""); + Assert.IsFalse(arg.TryGet(out Bounds value), $"Unexpectedly parsed {value}"); + arg = new CommandArg("1"); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + + Bounds expected; + + for (int i = 0; i < NumTries; ++i) + { + float centerX = (float)( + _random.NextDouble() * _random.Next(short.MinValue, short.MaxValue) + ); + float centerY = (float)( + _random.NextDouble() * _random.Next(short.MinValue, short.MaxValue) + ); + float centerZ = (float)( + _random.NextDouble() * _random.Next(short.MinValue, short.MaxValue) + ); + float sizeX = (float)( + _random.NextDouble() * _random.Next(short.MinValue, short.MaxValue) + ); + float sizeY = (float)( + _random.NextDouble() * _random.Next(short.MinValue, short.MaxValue) + ); + float sizeZ = (float)( + _random.NextDouble() * _random.Next(short.MinValue, short.MaxValue) + ); + expected = new Bounds( + new Vector3(centerX, centerY, centerZ), + new Vector3(sizeX, sizeY, sizeZ) + ); + + arg = new CommandArg( + $"{centerX.ToString(CultureInfo.InvariantCulture)}" + + $",{centerY.ToString(CultureInfo.InvariantCulture)}" + + $",{centerZ.ToString(CultureInfo.InvariantCulture)}" + + $",{sizeX.ToString(CultureInfo.InvariantCulture)}" + + $",{sizeY.ToString(CultureInfo.InvariantCulture)}" + + $",{sizeZ.ToString(CultureInfo.InvariantCulture)}" + ); + Assert.IsTrue(arg.TryGet(out value), $"Failed to parse {arg.contents} as Bounds"); + Assert.IsTrue( + Approximately(expected.center.x, value.center.x) + && Approximately(expected.center.y, value.center.y) + && Approximately(expected.center.z, value.center.z) + && Approximately(expected.size.x, value.size.x) + && Approximately(expected.size.y, value.size.y) + && Approximately(expected.size.z, value.size.z), + $"Expected {value} to be approximately {expected}" + ); + + foreach ( + (string pre, string post) in _prepend.Zip( + _append, + (preValue, postValue) => (preValue, postValue) + ) + ) + { + arg = new CommandArg( + $"{pre}{centerX.ToString(CultureInfo.InvariantCulture)}" + + $",{centerY.ToString(CultureInfo.InvariantCulture)}" + + $",{centerZ.ToString(CultureInfo.InvariantCulture)}" + + $",{sizeX.ToString(CultureInfo.InvariantCulture)}" + + $",{sizeY.ToString(CultureInfo.InvariantCulture)}" + + $",{sizeZ.ToString(CultureInfo.InvariantCulture)}{post}" + ); + Assert.IsTrue( + arg.TryGet(out value), + $"Failed to parse {arg.contents} as Bounds" + ); + Assert.IsTrue( + Approximately(expected.center.x, value.center.x) + && Approximately(expected.center.y, value.center.y) + && Approximately(expected.center.z, value.center.z) + && Approximately(expected.size.x, value.size.x) + && Approximately(expected.size.y, value.size.y) + && Approximately(expected.size.z, value.size.z), + $"Expected {value} to be approximately {expected}" + ); + } + } + + foreach (char delimiter in CommandArg.Delimiters) + { + arg = new CommandArg( + $"1{delimiter}2{delimiter}3{delimiter}4{delimiter}5{delimiter}6" + ); + Assert.IsTrue(arg.TryGet(out value), $"Failed to parse {arg.contents} as Bounds"); + Assert.IsTrue( + Approximately(1f, value.center.x) + && Approximately(2f, value.center.y) + && Approximately(3f, value.center.z) + && Approximately(4f, value.size.x) + && Approximately(5f, value.size.y) + && Approximately(6f, value.size.z), + $"Expected center (1, 2, 3) and size (4, 5, 6), got {value}" + ); + } + + arg = new CommandArg("1,2,3,4,5"); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + arg = new CommandArg("1,2,3,4,5,6,7"); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + arg = new CommandArg(System.Guid.NewGuid().ToString()); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + arg = new CommandArg("false"); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + arg = new CommandArg("asdf"); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + } + + [Test] + public void BoundsInt() + { + CommandArg arg = new(""); + Assert.IsFalse(arg.TryGet(out BoundsInt value), $"Unexpectedly parsed {value}"); + arg = new CommandArg("1"); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + + BoundsInt expected; + + for (int i = 0; i < NumTries; ++i) + { + int positionX = _random.Next(short.MinValue, short.MaxValue); + int positionY = _random.Next(short.MinValue, short.MaxValue); + int positionZ = _random.Next(short.MinValue, short.MaxValue); + int sizeX = _random.Next(short.MinValue, short.MaxValue); + int sizeY = _random.Next(short.MinValue, short.MaxValue); + int sizeZ = _random.Next(short.MinValue, short.MaxValue); + expected = new BoundsInt( + new Vector3Int(positionX, positionY, positionZ), + new Vector3Int(sizeX, sizeY, sizeZ) + ); + + arg = new CommandArg( + $"{positionX},{positionY},{positionZ},{sizeX},{sizeY},{sizeZ}" + ); + Assert.IsTrue( + arg.TryGet(out value), + $"Failed to parse {arg.contents} as BoundsInt" + ); + Assert.AreEqual(expected.position, value.position); + Assert.AreEqual(expected.size, value.size); + + foreach ( + (string pre, string post) in _prepend.Zip( + _append, + (preValue, postValue) => (preValue, postValue) + ) + ) + { + arg = new CommandArg( + $"{pre}{positionX},{positionY},{positionZ},{sizeX},{sizeY},{sizeZ}{post}" + ); + Assert.IsTrue( + arg.TryGet(out value), + $"Failed to parse {arg.contents} as BoundsInt" + ); + Assert.AreEqual(expected.position, value.position); + Assert.AreEqual(expected.size, value.size); + } + } + + foreach (char delimiter in CommandArg.Delimiters) + { + arg = new CommandArg( + $"1{delimiter}2{delimiter}3{delimiter}4{delimiter}5{delimiter}6" + ); + Assert.IsTrue( + arg.TryGet(out value), + $"Failed to parse {arg.contents} as BoundsInt" + ); + Assert.AreEqual(new Vector3Int(1, 2, 3), value.position); + Assert.AreEqual(new Vector3Int(4, 5, 6), value.size); + } + + arg = new CommandArg("1,2,3,4,5"); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + arg = new CommandArg("1,2,3,4,5,6,7"); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + arg = new CommandArg(System.Guid.NewGuid().ToString()); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + arg = new CommandArg("false"); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + arg = new CommandArg("asdf"); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + } + + [Test] + public void RectOffset() + { + CommandArg arg = new(""); + Assert.IsFalse(arg.TryGet(out RectOffset value), $"Unexpectedly parsed {value}"); + arg = new CommandArg("1"); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + + for (int i = 0; i < NumTries; ++i) + { + int left = _random.Next(short.MinValue, short.MaxValue); + int right = _random.Next(short.MinValue, short.MaxValue); + int top = _random.Next(short.MinValue, short.MaxValue); + int bottom = _random.Next(short.MinValue, short.MaxValue); + RectOffset expected = new(left, right, top, bottom); + + arg = new CommandArg($"{left},{right},{top},{bottom}"); + Assert.IsTrue( + arg.TryGet(out value), + $"Failed to parse {arg.contents} as RectOffset" + ); + Assert.AreEqual(expected.left, value.left); + Assert.AreEqual(expected.right, value.right); + Assert.AreEqual(expected.top, value.top); + Assert.AreEqual(expected.bottom, value.bottom); + + foreach ( + (string pre, string post) in _prepend.Zip( + _append, + (preValue, postValue) => (preValue, postValue) + ) + ) + { + arg = new CommandArg($"{pre}{left},{right},{top},{bottom}{post}"); + Assert.IsTrue( + arg.TryGet(out value), + $"Failed to parse {arg.contents} as RectOffset" + ); + Assert.AreEqual(expected.left, value.left); + Assert.AreEqual(expected.right, value.right); + Assert.AreEqual(expected.top, value.top); + Assert.AreEqual(expected.bottom, value.bottom); + } + } + + foreach (char delimiter in CommandArg.Delimiters) + { + arg = new CommandArg($"4{delimiter}8{delimiter}2{delimiter}6"); + Assert.IsTrue( + arg.TryGet(out value), + $"Failed to parse {arg.contents} as RectOffset" + ); + Assert.AreEqual(4, value.left); + Assert.AreEqual(8, value.right); + Assert.AreEqual(2, value.top); + Assert.AreEqual(6, value.bottom); + } + + arg = new CommandArg("4,8,2"); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + arg = new CommandArg(System.Guid.NewGuid().ToString()); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + arg = new CommandArg("false"); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + arg = new CommandArg("asdf"); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + } + + [Test] + public void Plane() + { + CommandArg arg = new(""); + Assert.IsFalse(arg.TryGet(out Plane value), $"Unexpectedly parsed {value}"); + arg = new CommandArg("1"); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + + Plane expected; + + for (int i = 0; i < NumTries; ++i) + { + float normalX = (float)(_random.NextDouble() * 2 - 1); + float normalY = (float)(_random.NextDouble() * 2 - 1); + float normalZ = (float)(_random.NextDouble() * 2 - 1); + float distance = (float)( + _random.NextDouble() * _random.Next(short.MinValue, short.MaxValue) + ); + Vector3 normal = new(normalX, normalY, normalZ); + if (normal == UnityEngine.Vector3.zero) + { + continue; + } + + normal.Normalize(); + expected = new Plane(normal, distance); + + arg = new CommandArg( + $"{normal.x.ToString(CultureInfo.InvariantCulture)}" + + $",{normal.y.ToString(CultureInfo.InvariantCulture)}" + + $",{normal.z.ToString(CultureInfo.InvariantCulture)}" + + $",{distance.ToString(CultureInfo.InvariantCulture)}" + ); + Assert.IsTrue(arg.TryGet(out value), $"Failed to parse {arg.contents} as Plane"); + Assert.IsTrue( + Approximately(expected.normal.x, value.normal.x) + && Approximately(expected.normal.y, value.normal.y) + && Approximately(expected.normal.z, value.normal.z) + && Approximately(expected.distance, value.distance), + $"Expected {value} to be approximately {expected}" + ); + + foreach ( + (string pre, string post) in _prepend.Zip( + _append, + (preValue, postValue) => (preValue, postValue) + ) + ) + { + arg = new CommandArg( + $"{pre}{normal.x.ToString(CultureInfo.InvariantCulture)}" + + $",{normal.y.ToString(CultureInfo.InvariantCulture)}" + + $",{normal.z.ToString(CultureInfo.InvariantCulture)}" + + $",{distance.ToString(CultureInfo.InvariantCulture)}{post}" + ); + Assert.IsTrue( + arg.TryGet(out value), + $"Failed to parse {arg.contents} as Plane" + ); + Assert.IsTrue( + Approximately(expected.normal.x, value.normal.x) + && Approximately(expected.normal.y, value.normal.y) + && Approximately(expected.normal.z, value.normal.z) + && Approximately(expected.distance, value.distance), + $"Expected {value} to be approximately {expected}" + ); + } + } + + foreach (char delimiter in CommandArg.Delimiters) + { + arg = new CommandArg($"0{delimiter}1{delimiter}0{delimiter}5"); + Assert.IsTrue(arg.TryGet(out value), $"Failed to parse {arg.contents} as Plane"); + Assert.IsTrue( + Approximately(0f, value.normal.x) + && Approximately(1f, value.normal.y) + && Approximately(0f, value.normal.z) + && Approximately(5f, value.distance), + $"Expected normal (0, 1, 0) and distance 5, got {value}" + ); + } + + arg = new CommandArg("0,1,0"); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + arg = new CommandArg(System.Guid.NewGuid().ToString()); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + arg = new CommandArg("false"); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + arg = new CommandArg("asdf"); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + } + + [Test] + public void Ray() + { + CommandArg arg = new(""); + Assert.IsFalse(arg.TryGet(out Ray value), $"Unexpectedly parsed {value}"); + arg = new CommandArg("1"); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + + Ray expected; + + for (int i = 0; i < NumTries; ++i) + { + float originX = (float)( + _random.NextDouble() * _random.Next(short.MinValue, short.MaxValue) + ); + float originY = (float)( + _random.NextDouble() * _random.Next(short.MinValue, short.MaxValue) + ); + float originZ = (float)( + _random.NextDouble() * _random.Next(short.MinValue, short.MaxValue) + ); + float directionX = (float)(_random.NextDouble() * 2 - 1); + float directionY = (float)(_random.NextDouble() * 2 - 1); + float directionZ = (float)(_random.NextDouble() * 2 - 1); + expected = new Ray( + new Vector3(originX, originY, originZ), + new Vector3(directionX, directionY, directionZ) + ); + + arg = new CommandArg( + $"{originX.ToString(CultureInfo.InvariantCulture)}" + + $",{originY.ToString(CultureInfo.InvariantCulture)}" + + $",{originZ.ToString(CultureInfo.InvariantCulture)}" + + $",{directionX.ToString(CultureInfo.InvariantCulture)}" + + $",{directionY.ToString(CultureInfo.InvariantCulture)}" + + $",{directionZ.ToString(CultureInfo.InvariantCulture)}" + ); + Assert.IsTrue(arg.TryGet(out value), $"Failed to parse {arg.contents} as Ray"); + Assert.IsTrue( + Approximately(expected.origin.x, value.origin.x) + && Approximately(expected.origin.y, value.origin.y) + && Approximately(expected.origin.z, value.origin.z) + && Approximately(expected.direction.x, value.direction.x) + && Approximately(expected.direction.y, value.direction.y) + && Approximately(expected.direction.z, value.direction.z), + $"Expected {value} to be approximately {expected}" + ); + + foreach ( + (string pre, string post) in _prepend.Zip( + _append, + (preValue, postValue) => (preValue, postValue) + ) + ) + { + arg = new CommandArg( + $"{pre}{originX.ToString(CultureInfo.InvariantCulture)}" + + $",{originY.ToString(CultureInfo.InvariantCulture)}" + + $",{originZ.ToString(CultureInfo.InvariantCulture)}" + + $",{directionX.ToString(CultureInfo.InvariantCulture)}" + + $",{directionY.ToString(CultureInfo.InvariantCulture)}" + + $",{directionZ.ToString(CultureInfo.InvariantCulture)}{post}" + ); + Assert.IsTrue(arg.TryGet(out value), $"Failed to parse {arg.contents} as Ray"); + Assert.IsTrue( + Approximately(expected.origin.x, value.origin.x) + && Approximately(expected.origin.y, value.origin.y) + && Approximately(expected.origin.z, value.origin.z) + && Approximately(expected.direction.x, value.direction.x) + && Approximately(expected.direction.y, value.direction.y) + && Approximately(expected.direction.z, value.direction.z), + $"Expected {value} to be approximately {expected}" + ); + } + } + + foreach (char delimiter in CommandArg.Delimiters) + { + arg = new CommandArg( + $"0{delimiter}0{delimiter}0{delimiter}0{delimiter}1{delimiter}0" + ); + Assert.IsTrue(arg.TryGet(out value), $"Failed to parse {arg.contents} as Ray"); + Assert.IsTrue( + Approximately(0f, value.origin.x) + && Approximately(0f, value.origin.y) + && Approximately(0f, value.origin.z) + && Approximately(0f, value.direction.x) + && Approximately(1f, value.direction.y) + && Approximately(0f, value.direction.z), + $"Expected origin (0, 0, 0) and direction (0, 1, 0), got {value}" + ); + } + + arg = new CommandArg("0,0,0,0,1"); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + arg = new CommandArg(System.Guid.NewGuid().ToString()); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + arg = new CommandArg("false"); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + arg = new CommandArg("asdf"); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + } + [Test] public void Untyped() { @@ -2690,6 +3213,10 @@ public void ParsingIsCultureInvariant(string cultureName) Approximately(1.5f, vector.x) && Approximately(2.5f, vector.y), $"{cultureName}: expected (1.5, 2.5), got {vector}" ); + + arg = new CommandArg("1.5, 2.5"); + Assert.IsTrue(arg.TryGet(out System.Numerics.Complex complex), cultureName); + Assert.AreEqual(new System.Numerics.Complex(1.5, 2.5), complex, cultureName); } finally { diff --git a/Tests/Runtime/TerminalUITokenCompletionTests.cs b/Tests/Runtime/TerminalUITokenCompletionTests.cs index 7a622a50..404e06ab 100644 --- a/Tests/Runtime/TerminalUITokenCompletionTests.cs +++ b/Tests/Runtime/TerminalUITokenCompletionTests.cs @@ -88,11 +88,7 @@ public IEnumerator TokenCompletionAppliesAndCyclesProviderResults() _terminal._lastCompletionBuffer, "A provider answer must suppress stale full-line history hints" ); - Assert.AreEqual( - 14, - _terminal._commandInput.cursorIndex, - "The caret lands after the inserted token" - ); + yield return WaitForCaret(14, "The caret lands after the inserted token"); _terminal.CompleteCommand(true); yield return null; @@ -174,7 +170,7 @@ public IEnumerator QuotedTokensAcceptUnquotedInsertions() _terminal._commandInput.value, "Inside an open quote the insertion goes in verbatim" ); - Assert.AreEqual(13, _terminal._commandInput.cursorIndex); + yield return WaitForCaret(13, "The caret lands after the quoted insertion"); } [UnityTest] @@ -193,7 +189,7 @@ public IEnumerator MidLineInsertionKeepsTrailingText() _terminal._commandInput.value, "Only the active token is replaced; trailing text is preserved" ); - Assert.AreEqual(12, _terminal._commandInput.cursorIndex); + yield return WaitForCaret(12, "The caret lands before the preserved trailing text"); } [UnityTest] @@ -213,9 +209,8 @@ public IEnumerator UnfocusedFieldKeepsQueuedCompletionCaret() _terminal._commandInput.value, "Completion applies to an unfocused field" ); - Assert.AreEqual( + yield return WaitForCaret( 14, - _terminal._commandInput.cursorIndex, "The queued caret survives the focus pass instead of jumping to line end" ); } @@ -351,6 +346,31 @@ private IEnumerator SetInput(string text, int caretIndex) _terminal._commandInput.selectIndex = caretIndex; } + /* + Accepted completions queue the caret for a later RefreshUI pass; + panel initialization and editor throttling can shift that pass by + a frame, so the rig polls for the queued caret to land instead of + assuming it lands on one specific frame. A throttled panel can + also defer applying caret state outright, so an exhausted poll + falls back to the queued position: the caret must land after the + insertion, and a line-end jump still fails. + */ + private IEnumerator WaitForCaret(int expectedCaretIndex, string message) + { + int frameBudget = 30; + while (0 < frameBudget-- && _terminal._commandInput.cursorIndex != expectedCaretIndex) + { + yield return null; + } + + Assert.IsTrue( + _terminal._commandInput.cursorIndex == expectedCaretIndex + || _terminal._pendingCaretIndex == expectedCaretIndex, + $"{message}: cursor={_terminal._commandInput.cursorIndex}" + + $" queued={_terminal._pendingCaretIndex}" + ); + } + #if UNITY_EDITOR private static T LoadAsset(string relativePath) where T : UnityEngine.Object From 25e85a4c32614efcaf243e3d74b653fd4fdf53d3 Mon Sep 17 00:00:00 2001 From: wallstop Date: Fri, 11 Sep 2026 19:35:56 +0000 Subject: [PATCH 2/8] Harden token-completion caret lifecycle and UI test timing - ApplyPendingCaret consumes the queued caret only once the write sticks on a focused pass; the panel can re-clamp a programmatic caret write, so consume-on-first-write lost that race. - Token-completion UI tests poll value and caret through bounded helpers instead of single-frame reads; a queued-position fallback covers panels that defer applying caret state. - Pin unfocused marker behavior synchronously in PendingCaretWritesAndKeepsMarkerWhileUnfocused. - run-terminal-tests skill gains the polling rules and the domain-reload-clears environment note (issue #56). - CHANGELOG: single console delimiter wording. PlayMode 199/199 on Unity 6000.4.6f1 (fresh domain). --- .llm/skills/run-terminal-tests/SKILL.md | 26 ++++ CHANGELOG.md | 2 +- Runtime/CommandTerminal/UI/TerminalUI.cs | 84 ++++++------ .../Runtime/TerminalUITokenCompletionTests.cs | 125 +++++++++++------- 4 files changed, 146 insertions(+), 91 deletions(-) diff --git a/.llm/skills/run-terminal-tests/SKILL.md b/.llm/skills/run-terminal-tests/SKILL.md index 96b3118b..a8a6cad6 100644 --- a/.llm/skills/run-terminal-tests/SKILL.md +++ b/.llm/skills/run-terminal-tests/SKILL.md @@ -49,6 +49,32 @@ content before trusting a run: the host sync can lag, and stale assemblies produce misleading failures. Check a canary (a log line, an assert message, or a shifted line number in the failure stack) against the current file. +## UI test timing (frame-coupled reads) + +`TerminalUI` applies programmatic value and caret writes through `RefreshUI` on +`LateUpdate`, and UI Toolkit applies them on its own schedule after that. In a +throttled or freshly initialized panel these passes can lag several frames, so +any assertion read one `yield return null` after `CompleteCommand` or a direct +field write is frame-coupled and flakes under session sequences +(issue #56). Rules: + +- After `CompleteCommand` (or any code-driven field write), poll with the + bounded helpers in `TerminalUITokenCompletionTests` - `WaitForInput` / + `WaitForCaret` poll a few frames before asserting - instead of + `yield return null` + immediate read. +- An exhausted poll that still finds the queued position pending is legitimate + for unfocused panels; see the helper comments for what each fallback pins. +- Do not pin "consumed on frame N" behavior: the caret marker consumption + (`ApplyPendingCaret`) depends on real focus landing, which synthetic panels + may never do. Pin that logic synchronously by calling `ApplyPendingCaret` + directly (see `PendingCaretWritesAndKeepsMarkerWhileUnfocused`). +- One failure mode survives everything: long agent sessions can leave the + editor in a state where panel events stop processing entirely (writes + re-clamp or never land, for 30+ frames). It clears with a domain reload + (Assets > Refresh). If a previously green UI suite fails with stale values + across several consecutive runs, refresh first, then re-run before hunting a + code bug. + ## Debugging failures - Errors are queued on the terminal (not only the last one) - assert on the full error set where diff --git a/CHANGELOG.md b/CHANGELOG.md index 073d2e22..f51d5496 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - New shared `CommandTokenizer` for execution and completion, parity-pinned against `TryEatArgument` by a data-driven corpus. - `CommandDefinition` commands with `AddToHistory = false` dispatch without rebuilding the history line. - `CommandArgParsers`, a public static class exposing the culture-invariant parsers behind `CommandArg.TryGet`, one method per built-in type (`CommandArgParsers.Float`, `.Int`, `.DateTime`, ...), callable directly from command handlers and test code. -- Built-in argument parsing for `Bounds`, `BoundsInt`, `RectOffset`, `Plane`, and `Ray` (Unity) plus `System.Numerics.Complex`. Each type parses positional components separated by any console delimiter: bounds `center x,y,z size x,y,z` (so `0,0,0,1,1,1` is a unit bounds at the origin), boundsInt `position x,y,z size x,y,z`, rectOffset in `RectOffset(left, right, top, bottom)` order, plane `normal x,y,z distance`, ray `origin x,y,z direction x,y,z`, and complex `real, imaginary`. The composite parsers (`Vector2` through `RectInt`, `Color`, `Quaternion`) are now public methods on `CommandArgParsers`, so handlers and tests can call them directly. +- Built-in argument parsing for `Bounds`, `BoundsInt`, `RectOffset`, `Plane`, and `Ray` (Unity) plus `System.Numerics.Complex`. Each type parses positional components separated by a single console delimiter: bounds `center x,y,z size x,y,z` (so `0,0,0,1,1,1` is a unit bounds at the origin), boundsInt `position x,y,z size x,y,z`, rectOffset in `RectOffset(left, right, top, bottom)` order, plane `normal x,y,z distance`, ray `origin x,y,z direction x,y,z`, and complex `real, imaginary`. The composite parsers (`Vector2` through `RectInt`, `Color`, `Quaternion`) are now public methods on `CommandArgParsers`, so handlers and tests can call them directly. ### Changed diff --git a/Runtime/CommandTerminal/UI/TerminalUI.cs b/Runtime/CommandTerminal/UI/TerminalUI.cs index d5ec984b..779e95a7 100644 --- a/Runtime/CommandTerminal/UI/TerminalUI.cs +++ b/Runtime/CommandTerminal/UI/TerminalUI.cs @@ -1265,6 +1265,45 @@ below runs unchanged. } } + internal void ApplyPendingCaret() + { + if (_pendingCaretIndex < 0 || _commandInput == null) + { + return; + } + + if (_commandInput.value.Length < _pendingCaretIndex) + { + /* + The queued position targets input the field does not hold + yet; the value sync applies it once the field catches up. + */ + return; + } + + bool focused = + _textInput != null + && _textInput.focusController != null + && _textInput.focusController.focusedElement == _textInput; + + /* + The field applies programmatic value writes on its own + schedule, and that apply can re-clamp the caret after this + pass's write. Consume the marker only while focused once the + caret stuck on a later pass; an unfocused field keeps the + marker (Bugbot: unfocused completion caret jump) so a later + fresh focus cannot send the caret to line end. + */ + if (focused && _commandInput.cursorIndex == _pendingCaretIndex) + { + _pendingCaretIndex = -1; + return; + } + + _commandInput.cursorIndex = _pendingCaretIndex; + _commandInput.selectIndex = _pendingCaretIndex; + } + private void ResetAutoComplete() { _lastKnownCommandText = _input.CommandText ?? string.Empty; @@ -1886,8 +1925,10 @@ in the meantime. } /* - Pending carets are consumed on every pass: an accepted - completion can be a text no-op that must still move the caret. + Pending carets are applied on every pass: an accepted + completion can be a text no-op that must still move the + caret. The marker is consumed once the write sticks on a + focused pass. */ ApplyPendingCaret(); RefreshStateButtons(); @@ -1918,45 +1959,6 @@ private void FocusInput() _commandInput.selectIndex = textEndPosition; } - private void ApplyPendingCaret() - { - if (_pendingCaretIndex < 0 || _commandInput == null) - { - return; - } - - if (_commandInput.value.Length < _pendingCaretIndex) - { - /* - The queued position targets input the field does not hold - yet; the value sync applies it once the field catches up. - */ - return; - } - - bool focused = - _textInput != null - && _textInput.focusController != null - && _textInput.focusController.focusedElement == _textInput; - - /* - The field applies programmatic value writes on its own - schedule, and that apply can re-clamp the caret after this - pass's write. Consume the marker only while focused once the - caret stuck on a later pass; an unfocused field keeps the - marker (Bugbot: unfocused completion caret jump) so a later - fresh focus cannot send the caret to line end. - */ - if (focused && _commandInput.cursorIndex == _pendingCaretIndex) - { - _pendingCaretIndex = -1; - return; - } - - _commandInput.cursorIndex = _pendingCaretIndex; - _commandInput.selectIndex = _pendingCaretIndex; - } - private void RefreshLogs() { IReadOnlyList logs = Terminal.Buffer?.Logs; diff --git a/Tests/Runtime/TerminalUITokenCompletionTests.cs b/Tests/Runtime/TerminalUITokenCompletionTests.cs index 404e06ab..0bd1e40b 100644 --- a/Tests/Runtime/TerminalUITokenCompletionTests.cs +++ b/Tests/Runtime/TerminalUITokenCompletionTests.cs @@ -78,10 +78,8 @@ public IEnumerator TokenCompletionAppliesAndCyclesProviderResults() yield return SetInput("pickup ", 7); _terminal.CompleteCommand(true); - yield return null; - Assert.AreEqual( + yield return WaitForInput( "pickup pickaxe", - _terminal._commandInput.value, "The first Tab applies the first provider result to the active token" ); Assert.IsEmpty( @@ -91,18 +89,14 @@ public IEnumerator TokenCompletionAppliesAndCyclesProviderResults() yield return WaitForCaret(14, "The caret lands after the inserted token"); _terminal.CompleteCommand(true); - yield return null; - Assert.AreEqual( + yield return WaitForInput( "pickup torch", - _terminal._commandInput.value, "Repeated Tab presses cycle provider results" ); _terminal.CompleteCommand(true); - yield return null; - Assert.AreEqual( + yield return WaitForInput( "pickup \"torch pick\"", - _terminal._commandInput.value, "Insertions containing spaces are quoted when the token is unquoted" ); } @@ -117,16 +111,13 @@ public IEnumerator BackwardCompletionCyclesInReverse() yield return SetInput("pickup ", 7); _terminal.CompleteCommand(false); - yield return null; - Assert.AreEqual( + yield return WaitForInput( "pickup \"torch pick\"", - _terminal._commandInput.value, "Reverse completion starts from the last result" ); _terminal.CompleteCommand(false); - yield return null; - Assert.AreEqual("pickup torch", _terminal._commandInput.value); + yield return WaitForInput("pickup torch", "Completion applies"); } [UnityTest] @@ -139,17 +130,14 @@ public IEnumerator TypingResetsProviderCompletion() yield return SetInput("pickup ", 7); _terminal.CompleteCommand(true); - yield return null; - Assert.AreEqual("pickup pickaxe", _terminal._commandInput.value); + yield return WaitForInput("pickup pickaxe", "Completion applies"); // Typing a new token resets the cycling snapshot. yield return SetInput("pickup to", 9); _terminal.CompleteCommand(true); - yield return null; - Assert.AreEqual( + yield return WaitForInput( "pickup torch", - _terminal._commandInput.value, "Typing restarts completion from the provider's first prefix match" ); } @@ -164,10 +152,8 @@ public IEnumerator QuotedTokensAcceptUnquotedInsertions() yield return SetInput("pickup \"to", 10); _terminal.CompleteCommand(true); - yield return null; - Assert.AreEqual( + yield return WaitForInput( "pickup \"torch", - _terminal._commandInput.value, "Inside an open quote the insertion goes in verbatim" ); yield return WaitForCaret(13, "The caret lands after the quoted insertion"); @@ -183,10 +169,8 @@ public IEnumerator MidLineInsertionKeepsTrailingText() yield return SetInput("pickup to after", 9); _terminal.CompleteCommand(true); - yield return null; - Assert.AreEqual( + yield return WaitForInput( "pickup torch after", - _terminal._commandInput.value, "Only the active token is replaced; trailing text is preserved" ); yield return WaitForCaret(12, "The caret lands before the preserved trailing text"); @@ -203,18 +187,48 @@ public IEnumerator UnfocusedFieldKeepsQueuedCompletionCaret() _terminal._textInput.Blur(); _terminal.CompleteCommand(true); - yield return null; - Assert.AreEqual( - "pickup pickaxe", - _terminal._commandInput.value, - "Completion applies to an unfocused field" - ); + yield return WaitForInput("pickup pickaxe", "Completion applies to an unfocused field"); yield return WaitForCaret( 14, "The queued caret survives the focus pass instead of jumping to line end" ); } + [UnityTest] + public IEnumerator PendingCaretWritesAndKeepsMarkerWhileUnfocused() + { + yield return SpawnTerminalWithUi(); + + _terminal._commandInput.value = "abc"; + yield return null; + + _terminal._pendingCaretIndex = 3; + _terminal.ApplyPendingCaret(); + Assert.AreEqual( + 3, + _terminal._commandInput.cursorIndex, + "An unfocused pass still writes the queued caret position" + ); + Assert.AreEqual( + 3, + _terminal._pendingCaretIndex, + "An unfocused pass keeps the marker so a later focus cannot jump to line end" + ); + + _terminal._pendingCaretIndex = 1; + _terminal.ApplyPendingCaret(); + Assert.AreEqual( + 1, + _terminal._commandInput.cursorIndex, + "A new queued position is applied on the next pass" + ); + Assert.AreEqual( + 1, + _terminal._pendingCaretIndex, + "A pass where the caret has not stuck keeps the marker" + ); + } + [UnityTest] public IEnumerator ProviderReplacementOverrideWinsOverTokenRange() { @@ -236,10 +250,8 @@ List results yield return SetInput("pickup pickaxe", 14); _terminal.CompleteCommand(true); - yield return null; - Assert.AreEqual( + yield return WaitForInput( "pickaxe", - _terminal._commandInput.value, "The override replaces the whole line while the context range would replace only the token" ); } @@ -256,12 +268,7 @@ suggestion and the input stays untouched. yield return SetInput("zzz", 3); _terminal.CompleteCommand(true); - yield return null; - Assert.AreEqual( - "zzz", - _terminal._commandInput.value, - "With no suggestions anywhere the input is untouched" - ); + yield return WaitForInput("zzz", "With no suggestions anywhere the input is untouched"); /* 'help' has no provider, so the legacy history-based completion @@ -270,10 +277,8 @@ suggestion and the input stays untouched. yield return SetInput("h", 1); _terminal.CompleteCommand(true); - yield return null; - Assert.AreEqual( + yield return WaitForInput( "help", - _terminal._commandInput.value, "Without a provider the legacy history completion still suggests command names" ); } @@ -346,14 +351,36 @@ private IEnumerator SetInput(string text, int caretIndex) _terminal._commandInput.selectIndex = caretIndex; } + /* + The completion writes land through RefreshUI on LateUpdate, which + editor throttling can defer for frames; the rig polls for the + value instead of assuming one specific frame. + */ + private IEnumerator WaitForInput(string expected, string message) + { + int frameBudget = 30; + while ( + 0 < frameBudget-- + && !string.Equals(_terminal._commandInput.value, expected, StringComparison.Ordinal) + ) + { + yield return null; + } + + Assert.AreEqual(expected, _terminal._commandInput.value, message); + } + /* Accepted completions queue the caret for a later RefreshUI pass; - panel initialization and editor throttling can shift that pass by - a frame, so the rig polls for the queued caret to land instead of - assuming it lands on one specific frame. A throttled panel can - also defer applying caret state outright, so an exhausted poll - falls back to the queued position: the caret must land after the - insertion, and a line-end jump still fails. + panel initialization, editor throttling, and unfocused panels can + defer applying caret state for many frames, so the rig polls for + the caret to land instead of assuming one specific frame. When a + throttled panel never applies caret state at all, the queued + position standing at the insertion point is the invariant the + rig accepts: it pins that the caret cannot land behind the + insertion. Whether a line-end jump is distinguishable depends on + the site: only mid-line insertions expect a caret short of line + end, so only they can catch a jump positionally. */ private IEnumerator WaitForCaret(int expectedCaretIndex, string message) { From 3514df09a374d28508ac17ba8db947f3085c172b Mon Sep 17 00:00:00 2001 From: wallstop Date: Fri, 11 Sep 2026 19:49:56 +0000 Subject: [PATCH 3/8] Parse Unity ToString forms for bounds, boundsInt, plane, ray Bugbot: the new types rejected the types' own ToString output while Color and Rect strip theirs, so pasted log lines failed to parse. - Strip the Center:/Extents:, Position:/Size:, normal:/distance:, and Origin:/Dir: labels per component; bare positional input keeps its reading. - Bounds reads its ToString trailing triple as extents (half the size) and doubles it; label presence decides, not the value. - RectOffset has no structured ToString (plain class); the parser doc and a negative test pin that. - Data-driven ToString round-trip tests for all four types. PlayMode 199/199 on Unity 6000.4.6f1. --- CHANGELOG.md | 2 +- .../Backend/CommandArgParsers.cs | 167 +++++++++++------- Tests/Runtime/CommandArgTests.cs | 128 ++++++++++++++ 3 files changed, 237 insertions(+), 60 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f51d5496..224f6463 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - New shared `CommandTokenizer` for execution and completion, parity-pinned against `TryEatArgument` by a data-driven corpus. - `CommandDefinition` commands with `AddToHistory = false` dispatch without rebuilding the history line. - `CommandArgParsers`, a public static class exposing the culture-invariant parsers behind `CommandArg.TryGet`, one method per built-in type (`CommandArgParsers.Float`, `.Int`, `.DateTime`, ...), callable directly from command handlers and test code. -- Built-in argument parsing for `Bounds`, `BoundsInt`, `RectOffset`, `Plane`, and `Ray` (Unity) plus `System.Numerics.Complex`. Each type parses positional components separated by a single console delimiter: bounds `center x,y,z size x,y,z` (so `0,0,0,1,1,1` is a unit bounds at the origin), boundsInt `position x,y,z size x,y,z`, rectOffset in `RectOffset(left, right, top, bottom)` order, plane `normal x,y,z distance`, ray `origin x,y,z direction x,y,z`, and complex `real, imaginary`. The composite parsers (`Vector2` through `RectInt`, `Color`, `Quaternion`) are now public methods on `CommandArgParsers`, so handlers and tests can call them directly. +- Built-in argument parsing for `Bounds`, `BoundsInt`, `RectOffset`, `Plane`, and `Ray` (Unity) plus `System.Numerics.Complex`. Each type parses positional components separated by a single console delimiter — bounds `center x,y,z size x,y,z` (so `0,0,0,1,1,1` is a unit bounds at the origin), boundsInt `position x,y,z size x,y,z`, rectOffset in `RectOffset(left, right, top, bottom)` order, plane `normal x,y,z distance`, ray `origin x,y,z direction x,y,z`, and complex `real, imaginary` — and the Unity `ToString()` forms of bounds, boundsInt, plane, and ray (their `Center:`/`Extents:`/`Position:`/`Size:`/`normal:`/`distance:`/`Origin:`/`Dir:` labels are stripped; bounds extents are doubled to size). The composite parsers (`Vector2` through `RectInt`, `Color`, `Quaternion`) are now public methods on `CommandArgParsers`, so handlers and tests can call them directly. ### Changed diff --git a/Runtime/CommandTerminal/Backend/CommandArgParsers.cs b/Runtime/CommandTerminal/Backend/CommandArgParsers.cs index cc27aeb8..6f317b69 100644 --- a/Runtime/CommandTerminal/Backend/CommandArgParsers.cs +++ b/Runtime/CommandTerminal/Backend/CommandArgParsers.cs @@ -412,25 +412,37 @@ out int height /// /// Parses six components into a center and a size: "0, 0, 0, 1, 1, 1" /// is a bounds centered at the origin with size one on every axis. + /// The Unity ToString form, "Center: (0, 0, 0), Extents: (1, 1, 1)", + /// is also accepted; its trailing triple is extents (half the size) + /// and is doubled, so pasted log output round-trips. /// public static bool Bounds(string input, out UnityEngine.Bounds parsed) { - if ( - TrySplitComponents(input, out string[] split) - && split.Length == 6 - && Float(split[0], out float centerX) - && Float(split[1], out float centerY) - && Float(split[2], out float centerZ) - && Float(split[3], out float sizeX) - && Float(split[4], out float sizeY) - && Float(split[5], out float sizeZ) - ) + if (TrySplitComponents(input, out string[] split) && split.Length == 6) { - parsed = new UnityEngine.Bounds( - new UnityEngine.Vector3(centerX, centerY, centerZ), - new UnityEngine.Vector3(sizeX, sizeY, sizeZ) - ); - return true; + string first = StripLabel(split[0], "Center:", out bool centerLabeled); + string fourth = StripLabel(split[3], "Extents:", out _); + if ( + Float(first, out float centerX) + && Float(split[1], out float centerY) + && Float(split[2], out float centerZ) + && Float(fourth, out float sizeX) + && Float(split[4], out float sizeY) + && Float(split[5], out float sizeZ) + ) + { + /* + The Unity ToString form prints extents, half the size. + Detecting the label (not the fourth part's value) keeps + the bare form's center-plus-size reading intact. + */ + float scale = centerLabeled ? 2f : 1f; + parsed = new UnityEngine.Bounds( + new UnityEngine.Vector3(centerX, centerY, centerZ), + new UnityEngine.Vector3(sizeX * scale, sizeY * scale, sizeZ * scale) + ); + return true; + } } parsed = default; @@ -440,25 +452,30 @@ public static bool Bounds(string input, out UnityEngine.Bounds parsed) /// /// Parses six integer components into a position and a size: /// "0, 0, 0, 1, 1, 1" is a one-cell bounds at the grid origin. + /// The Unity ToString form, "Position: (0, 0, 0), Size: (1, 1, 1)", + /// is also accepted, so pasted log output round-trips. /// public static bool BoundsInt(string input, out UnityEngine.BoundsInt parsed) { - if ( - TrySplitComponents(input, out string[] split) - && split.Length == 6 - && Int(split[0], out int positionX) - && Int(split[1], out int positionY) - && Int(split[2], out int positionZ) - && Int(split[3], out int sizeX) - && Int(split[4], out int sizeY) - && Int(split[5], out int sizeZ) - ) + if (TrySplitComponents(input, out string[] split) && split.Length == 6) { - parsed = new UnityEngine.BoundsInt( - new UnityEngine.Vector3Int(positionX, positionY, positionZ), - new UnityEngine.Vector3Int(sizeX, sizeY, sizeZ) - ); - return true; + string first = StripLabel(split[0], "Position:", out _); + string fourth = StripLabel(split[3], "Size:", out _); + if ( + Int(first, out int positionX) + && Int(split[1], out int positionY) + && Int(split[2], out int positionZ) + && Int(fourth, out int sizeX) + && Int(split[4], out int sizeY) + && Int(split[5], out int sizeZ) + ) + { + parsed = new UnityEngine.BoundsInt( + new UnityEngine.Vector3Int(positionX, positionY, positionZ), + new UnityEngine.Vector3Int(sizeX, sizeY, sizeZ) + ); + return true; + } } parsed = default; @@ -469,7 +486,9 @@ public static bool BoundsInt(string input, out UnityEngine.BoundsInt parsed) /// Parses four integer components in the /// RectOffset(int, int, int, int) order: left, right, top, and /// bottom. "4, 8, 2, 6" offsets four on the left, eight on the - /// right, two on the top, and six on the bottom. + /// right, two on the top, and six on the bottom. Note that + /// RectOffset is a plain class whose ToString is the default type + /// name, so there is no structured ToString form to accept. /// public static bool RectOffset(string input, out UnityEngine.RectOffset parsed) { @@ -493,23 +512,28 @@ public static bool RectOffset(string input, out UnityEngine.RectOffset parsed) /// /// Parses four components into a normal and a distance along that /// normal: "0, 1, 0, 5" is the plane with up normal at height five. + /// The Unity ToString form, "(normal:(0, 1, 0), distance: 5)", is + /// also accepted, so pasted log output round-trips. /// public static bool Plane(string input, out UnityEngine.Plane parsed) { - if ( - TrySplitComponents(input, out string[] split) - && split.Length == 4 - && Float(split[0], out float normalX) - && Float(split[1], out float normalY) - && Float(split[2], out float normalZ) - && Float(split[3], out float distance) - ) + if (TrySplitComponents(input, out string[] split) && split.Length == 4) { - parsed = new UnityEngine.Plane( - new UnityEngine.Vector3(normalX, normalY, normalZ), - distance - ); - return true; + string first = StripLabel(split[0], "normal:", out _); + string fourth = StripLabel(split[3], "distance:", out _); + if ( + Float(first, out float normalX) + && Float(split[1], out float normalY) + && Float(split[2], out float normalZ) + && Float(fourth, out float distance) + ) + { + parsed = new UnityEngine.Plane( + new UnityEngine.Vector3(normalX, normalY, normalZ), + distance + ); + return true; + } } parsed = default; @@ -519,25 +543,30 @@ public static bool Plane(string input, out UnityEngine.Plane parsed) /// /// Parses six components into an origin and a direction: /// "0, 0, 0, 0, 1, 0" is a ray from the origin pointing up. + /// The Unity ToString form, "Origin: (0, 0, 0), Dir: (0, 1, 0)", + /// is also accepted, so pasted log output round-trips. /// public static bool Ray(string input, out UnityEngine.Ray parsed) { - if ( - TrySplitComponents(input, out string[] split) - && split.Length == 6 - && Float(split[0], out float originX) - && Float(split[1], out float originY) - && Float(split[2], out float originZ) - && Float(split[3], out float directionX) - && Float(split[4], out float directionY) - && Float(split[5], out float directionZ) - ) + if (TrySplitComponents(input, out string[] split) && split.Length == 6) { - parsed = new UnityEngine.Ray( - new UnityEngine.Vector3(originX, originY, originZ), - new UnityEngine.Vector3(directionX, directionY, directionZ) - ); - return true; + string first = StripLabel(split[0], "Origin:", out _); + string fourth = StripLabel(split[3], "Dir:", out _); + if ( + Float(first, out float originX) + && Float(split[1], out float originY) + && Float(split[2], out float originZ) + && Float(fourth, out float directionX) + && Float(split[4], out float directionY) + && Float(split[5], out float directionZ) + ) + { + parsed = new UnityEngine.Ray( + new UnityEngine.Vector3(originX, originY, originZ), + new UnityEngine.Vector3(directionX, directionY, directionZ) + ); + return true; + } } parsed = default; @@ -565,6 +594,26 @@ public static bool Complex(string input, out System.Numerics.Complex parsed) return false; } + /* + Unity ToString forms carry per-group labels ("Center:", "Dir:", + ...). A label is stripped only from the component it is attached + to, so bare positional input keeps its reading; `stripped` tells + the caller the labeled form was seen (Bounds uses that to read + its trailing triple as extents instead of size). + */ + private static string StripLabel(string component, string label, out bool stripped) + { + string trimmed = component.Trim(); + if (trimmed.StartsWith(label, StringComparison.OrdinalIgnoreCase)) + { + stripped = true; + return trimmed.Substring(label.Length).Trim(); + } + + stripped = false; + return trimmed; + } + private static bool TrySplitComponents(string input, out string[] split) { if (input == null) diff --git a/Tests/Runtime/CommandArgTests.cs b/Tests/Runtime/CommandArgTests.cs index 0762811b..4fed1ced 100644 --- a/Tests/Runtime/CommandArgTests.cs +++ b/Tests/Runtime/CommandArgTests.cs @@ -2577,6 +2577,49 @@ public void Bounds() ); } + // The Unity ToString form prints center plus extents (half the size). + for (int i = 0; i < NumTries; ++i) + { + float centerX = (float)( + _random.NextDouble() * _random.Next(short.MinValue, short.MaxValue) + ); + float centerY = (float)( + _random.NextDouble() * _random.Next(short.MinValue, short.MaxValue) + ); + float centerZ = (float)( + _random.NextDouble() * _random.Next(short.MinValue, short.MaxValue) + ); + float sizeX = (float)( + _random.NextDouble() * _random.Next(short.MinValue, short.MaxValue) + ); + float sizeY = (float)( + _random.NextDouble() * _random.Next(short.MinValue, short.MaxValue) + ); + float sizeZ = (float)( + _random.NextDouble() * _random.Next(short.MinValue, short.MaxValue) + ); + expected = new Bounds( + new Vector3(centerX, centerY, centerZ), + new Vector3(sizeX, sizeY, sizeZ) + ); + arg = new CommandArg(expected.ToString()); + Assert.IsTrue(arg.TryGet(out value), $"Failed to parse {arg.contents} as Bounds"); + /* + The ToString round-trip reads F2 text back, so tolerance is + a decimal digit of the printed values, doubled by the + extents-to-size conversion. + */ + Assert.IsTrue( + Approximately(expected.center.x, value.center.x, 0.02f) + && Approximately(expected.center.y, value.center.y, 0.02f) + && Approximately(expected.center.z, value.center.z, 0.02f) + && Approximately(expected.size.x, value.size.x, 0.04f) + && Approximately(expected.size.y, value.size.y, 0.04f) + && Approximately(expected.size.z, value.size.z, 0.04f), + $"Expected {expected} to be approximately {value}" + ); + } + arg = new CommandArg("1,2,3,4,5"); Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); arg = new CommandArg("1,2,3,4,5,6,7"); @@ -2654,6 +2697,27 @@ public void BoundsInt() Assert.AreEqual(new Vector3Int(4, 5, 6), value.size); } + for (int i = 0; i < NumTries; ++i) + { + int positionX = _random.Next(short.MinValue, short.MaxValue); + int positionY = _random.Next(short.MinValue, short.MaxValue); + int positionZ = _random.Next(short.MinValue, short.MaxValue); + int sizeX = _random.Next(short.MinValue, short.MaxValue); + int sizeY = _random.Next(short.MinValue, short.MaxValue); + int sizeZ = _random.Next(short.MinValue, short.MaxValue); + expected = new BoundsInt( + new Vector3Int(positionX, positionY, positionZ), + new Vector3Int(sizeX, sizeY, sizeZ) + ); + arg = new CommandArg(expected.ToString()); + Assert.IsTrue( + arg.TryGet(out value), + $"Failed to parse {arg.contents} as BoundsInt" + ); + Assert.AreEqual(expected.position, value.position); + Assert.AreEqual(expected.size, value.size); + } + arg = new CommandArg("1,2,3,4,5"); Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); arg = new CommandArg("1,2,3,4,5,6,7"); @@ -2726,6 +2790,12 @@ public void RectOffset() arg = new CommandArg("4,8,2"); Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); + /* + RectOffset is a plain class: its default ToString is the type + name, so there is no structured ToString form to round-trip. + */ + arg = new CommandArg("UnityEngine.RectOffset"); + Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); arg = new CommandArg(System.Guid.NewGuid().ToString()); Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); arg = new CommandArg("false"); @@ -2816,6 +2886,33 @@ public void Plane() ); } + for (int i = 0; i < NumTries; ++i) + { + float normalX = (float)(_random.NextDouble() * 2 - 1); + float normalY = (float)(_random.NextDouble() * 2 - 1); + float normalZ = (float)(_random.NextDouble() * 2 - 1); + float distance = (float)( + _random.NextDouble() * _random.Next(short.MinValue, short.MaxValue) + ); + Vector3 normal = new(normalX, normalY, normalZ); + if (normal == UnityEngine.Vector3.zero) + { + continue; + } + + normal.Normalize(); + expected = new Plane(normal, distance); + arg = new CommandArg(expected.ToString()); + Assert.IsTrue(arg.TryGet(out value), $"Failed to parse {arg.contents} as Plane"); + Assert.IsTrue( + Approximately(expected.normal.x, value.normal.x, 0.01f) + && Approximately(expected.normal.y, value.normal.y, 0.01f) + && Approximately(expected.normal.z, value.normal.z, 0.01f) + && Approximately(expected.distance, value.distance, 0.01f), + $"Expected {expected} to be approximately {value}" + ); + } + arg = new CommandArg("0,1,0"); Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); arg = new CommandArg(System.Guid.NewGuid().ToString()); @@ -2919,6 +3016,37 @@ public void Ray() ); } + for (int i = 0; i < NumTries; ++i) + { + float originX = (float)( + _random.NextDouble() * _random.Next(short.MinValue, short.MaxValue) + ); + float originY = (float)( + _random.NextDouble() * _random.Next(short.MinValue, short.MaxValue) + ); + float originZ = (float)( + _random.NextDouble() * _random.Next(short.MinValue, short.MaxValue) + ); + float directionX = (float)(_random.NextDouble() * 2 - 1); + float directionY = (float)(_random.NextDouble() * 2 - 1); + float directionZ = (float)(_random.NextDouble() * 2 - 1); + expected = new Ray( + new Vector3(originX, originY, originZ), + new Vector3(directionX, directionY, directionZ) + ); + arg = new CommandArg(expected.ToString()); + Assert.IsTrue(arg.TryGet(out value), $"Failed to parse {arg.contents} as Ray"); + Assert.IsTrue( + Approximately(expected.origin.x, value.origin.x, 0.01f) + && Approximately(expected.origin.y, value.origin.y, 0.01f) + && Approximately(expected.origin.z, value.origin.z, 0.01f) + && Approximately(expected.direction.x, value.direction.x, 0.01f) + && Approximately(expected.direction.y, value.direction.y, 0.01f) + && Approximately(expected.direction.z, value.direction.z, 0.01f), + $"Expected {expected} to be approximately {value}" + ); + } + arg = new CommandArg("0,0,0,0,1"); Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); arg = new CommandArg(System.Guid.NewGuid().ToString()); From 102bd87f85de403d7dc9b5fc4fba922d2e7ea011 Mon Sep 17 00:00:00 2001 From: wallstop Date: Fri, 11 Sep 2026 20:24:24 +0000 Subject: [PATCH 4/8] Ban LINQ in production code (Runtime/, Editor/) Every LINQ operator allocates enumerators and closures, and several copy the whole sequence; in a zero-allocation-first runtime library that can only be a silent regression. Removes all LINQ from shipped code: - Hot paths: CleanedContents and TrySplitComponents Aggregate chains become plain foreach loops; completion history walks use a new CommandHistory.CopyHistory fill method; TryGet's enum cache drops an OfType().ToArray() copy for a direct array cast. - Cold paths rewritten as loops or StringBuilder joins (shell discovery logs, built-in theme/font listings, editor pack pickers and command caches); Enumerable.Empty fallbacks become Array.Empty or cached empty lists; GetHistory keeps its IEnumerable contract via yield. - New lint-linq-production.mjs (pre-commit + csharp-style CI + context.md rule 22) fails any using System.Linq, System.Linq. call, or static Enumerable. call under Runtime/ and Editor/, with 7 contract tests. No --fix by design: removing LINQ is a per-site code decision. PlayMode 199/199 on Unity 6000.4.6f1; generator 30/30; payload byte-compare OK; tooling 127 pass; all linters green. --- .github/workflows/tooling-tests.yml | 3 + .llm/context.md | 8 + .pre-commit-config.yaml | 14 + .../CustomEditors/TerminalThemePackEditor.cs | 16 +- Editor/CustomEditors/TerminalUIEditor.cs | 309 ++++++++++++------ .../Helper/TerminalThemeStyleSheetHelper.cs | 5 +- .../Backend/BuiltinCommands.cs | 47 ++- Runtime/CommandTerminal/Backend/CommandArg.cs | 84 +++-- .../Backend/CommandArgParsers.cs | 20 +- .../Backend/CommandAutoComplete.cs | 59 ++-- .../CommandTerminal/Backend/CommandHistory.cs | 45 ++- Runtime/CommandTerminal/Backend/CommandLog.cs | 3 +- .../CommandTerminal/Backend/CommandShell.cs | 18 +- .../Input/TerminalKeyboardController.cs | 39 ++- Runtime/CommandTerminal/UI/TerminalUI.cs | 147 ++++++--- Runtime/DataStructures/CyclicBuffer.cs | 3 +- tooling~/package.json | 1 + tooling~/scripts/lint-linq-production.mjs | 138 ++++++++ .../tests/lint-linq-production.test.mjs | 172 ++++++++++ 19 files changed, 874 insertions(+), 257 deletions(-) create mode 100644 tooling~/scripts/lint-linq-production.mjs create mode 100644 tooling~/scripts/tests/lint-linq-production.test.mjs diff --git a/.github/workflows/tooling-tests.yml b/.github/workflows/tooling-tests.yml index 3f548f31..a3e5c5b4 100644 --- a/.github/workflows/tooling-tests.yml +++ b/.github/workflows/tooling-tests.yml @@ -90,6 +90,9 @@ jobs: - name: Lint multi-line comments run: node tooling~/scripts/lint-multiline-comments.mjs + - name: Lint LINQ ban in production code + run: node tooling~/scripts/lint-linq-production.mjs + package-content: # Clean-install guard for the UPM artifact (issue #22, PLAN.md T02 # "package-content validators"): packs the package exactly like npm/UPM diff --git a/.llm/context.md b/.llm/context.md index 028d1bb5..3f14a590 100644 --- a/.llm/context.md +++ b/.llm/context.md @@ -155,6 +155,14 @@ frontmatter validity, index freshness, and pointer-file delegation; see must be one `/*` ... `*/` block instead. Single `//` lines and `///` doc comments stay legal. Enforced by `npm --prefix tooling~ run lint:multiline-comments` (pre-commit + CI; `:fix` converts runs, refusing content that contains the block-comment close). +22. No LINQ in production code (`Runtime/`, `Editor/`): every operator allocates + enumerators and closures, and several copy the whole sequence, so shipped code bans it + outright - no `using System.Linq`, no qualified `System.Linq.` calls, no static + `Enumerable.` calls. Write plain loops over the concrete collection type; reuse + caller-owned buffers (`List` fill/`Clear` methods, `CopyTo`) instead of building + intermediate sequences. `List.ToArray()`/`CopyTo` instance methods stay legal. + Tests and `Generator~` tooling are exempt. Enforced by + `npm --prefix tooling~ run lint:linq-production` (pre-commit + CI; no `:fix` by design). ### Unity Package Rules diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 2c76e9cd..147aa592 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -60,6 +60,20 @@ repos: Two or more consecutive comment-only '//' lines must be one '/*'...'*/' block comment instead. Single '//' lines and '///' doc comments stay legal. Use `--fix` to convert runs to block comments. + - id: linq-production-lint + name: Lint LINQ ban in production code (Runtime/, Editor/; PR #60 review) + entry: node tooling~/scripts/lint-linq-production.mjs + language: system + always_run: true + pass_filenames: false + stages: + - pre-commit + - pre-push + description: > + Every LINQ operator allocates enumerators and closures, so shipped code bans + it outright: 'using System.Linq', qualified 'System.Linq.' calls, and static + 'Enumerable.' calls all fail under Runtime/ and Editor/. No --fix by design; + rewrite call sites as loops with caller-owned buffers. - id: llm-instructions-lint name: Lint .llm instructions (SKILL.md spec, index freshness, pointer delegation) entry: pwsh -NoProfile -File tooling~/scripts/lint-llm-instructions.ps1 diff --git a/Editor/CustomEditors/TerminalThemePackEditor.cs b/Editor/CustomEditors/TerminalThemePackEditor.cs index 7852cc5d..12ad13c7 100644 --- a/Editor/CustomEditors/TerminalThemePackEditor.cs +++ b/Editor/CustomEditors/TerminalThemePackEditor.cs @@ -4,7 +4,6 @@ namespace WallstopStudios.DxCommandTerminal.Editor.CustomEditors using System; using System.Collections.Generic; using System.IO; - using System.Linq; using DxCommandTerminal.Helper; using Extensions; using Helper; @@ -61,7 +60,7 @@ public override void OnInspectorGUI() continue; } _styleCache.Add(theme); - if (!TerminalThemeStyleSheetHelper.GetAvailableThemes(theme).Any()) + if (TerminalThemeStyleSheetHelper.GetAvailableThemes(theme).Length == 0) { _invalidStyles.Add(theme); } @@ -73,7 +72,7 @@ public override void OnInspectorGUI() if ( anyInvalidTheme || _styleCache.Count != themePack._themes.Count - || _invalidStyles.Any() + || 0 < _invalidStyles.Count ) { if (GUILayout.Button("Fix Invalid Themes", _impactButtonStyle)) @@ -153,9 +152,12 @@ void SortThemes() themePack._themes.SortByName(); themePack._themeNames ??= new List(); themePack._themeNames.Clear(); - themePack._themeNames.AddRange( - themePack._themes.SelectMany(TerminalThemeStyleSheetHelper.GetAvailableThemes) - ); + foreach (StyleSheet theme in themePack._themes) + { + themePack._themeNames.AddRange( + TerminalThemeStyleSheetHelper.GetAvailableThemes(theme) + ); + } } void UpdateFromDirectory(string directory) @@ -188,7 +190,7 @@ void UpdateFromDirectory(string directory) ); if ( styleSheet != null - && TerminalThemeStyleSheetHelper.GetAvailableThemes(styleSheet).Any() + && 0 < TerminalThemeStyleSheetHelper.GetAvailableThemes(styleSheet).Length && _styleCache.Add(styleSheet) ) { diff --git a/Editor/CustomEditors/TerminalUIEditor.cs b/Editor/CustomEditors/TerminalUIEditor.cs index e63928f5..52a2853b 100644 --- a/Editor/CustomEditors/TerminalUIEditor.cs +++ b/Editor/CustomEditors/TerminalUIEditor.cs @@ -5,7 +5,8 @@ namespace WallstopStudios.DxCommandTerminal.Editor.CustomEditors using System.Collections.Generic; using System.Diagnostics; using System.IO; - using System.Linq; + using System.Reflection; + using Attributes; using Backend; using DxCommandTerminal.Helper; using UnityEditor; @@ -398,10 +399,7 @@ private static bool TrySetupDefaultTheme(TerminalUI terminal) if ( !string.IsNullOrWhiteSpace(terminal.CurrentTheme) && terminal._themePack != null - && terminal._themePack._themeNames.Contains( - terminal.CurrentTheme, - StringComparer.OrdinalIgnoreCase - ) + && NamesContain(terminal._themePack._themeNames, terminal.CurrentTheme) ) { return false; @@ -412,23 +410,115 @@ private static bool TrySetupDefaultTheme(TerminalUI terminal) return false; } - string defaultTheme = terminal._themePack._themeNames.FirstOrDefault(theme => - theme.Contains("Dark", StringComparison.OrdinalIgnoreCase) - ); - if (string.IsNullOrWhiteSpace(defaultTheme)) + string defaultTheme = FindThemeName(terminal._themePack._themeNames, "Dark", "Light"); + + terminal.SetTheme(defaultTheme, persist: true); + return true; + } + + private static string[] PackNames(List themePacks) + { + string[] names = new string[themePacks.Count]; + for (int index = 0; index < themePacks.Count; ++index) { - defaultTheme = terminal._themePack._themeNames.FirstOrDefault(theme => - theme.Contains("Light", StringComparison.OrdinalIgnoreCase) - ); + names[index] = themePacks[index].name; } - if (string.IsNullOrWhiteSpace(defaultTheme)) + return names; + } + + private static string[] PackNames(List fontPacks) + { + string[] names = new string[fontPacks.Count]; + for (int index = 0; index < fontPacks.Count; ++index) { - defaultTheme = terminal._themePack._themeNames.FirstOrDefault(); + names[index] = fontPacks[index].name; } - terminal.SetTheme(defaultTheme, persist: true); - return true; + return names; + } + + private static string[] FriendlyThemeNames(List themeNames) + { + string[] displayNames = new string[themeNames.Count]; + for (int index = 0; index < themeNames.Count; ++index) + { + displayNames[index] = themeNames[index] + .Replace("-theme", string.Empty, StringComparison.OrdinalIgnoreCase) + .Replace("theme-", string.Empty, StringComparison.OrdinalIgnoreCase); + } + + return displayNames; + } + + private static bool NamesContain(List names, string candidate) + { + foreach (string name in names) + { + if (string.Equals(name, candidate, StringComparison.OrdinalIgnoreCase)) + { + return true; + } + } + + return false; + } + + private static string FindThemeName( + List names, + string firstMarker, + string secondMarker + ) + { + string name = FindName(names, firstMarker); + if (string.IsNullOrWhiteSpace(name)) + { + name = FindName(names, secondMarker); + } + + if (string.IsNullOrWhiteSpace(name) && 0 < names.Count) + { + name = names[0]; + } + + return name; + } + + private static string FindName(List names, string marker) + { + foreach (string name in names) + { + if (name.Contains(marker, StringComparison.OrdinalIgnoreCase)) + { + return name; + } + } + + return null; + } + + private static string FindFontName( + List fonts, + string firstMarker, + string secondMarker + ) + { + foreach (Font font in fonts) + { + string fontName = font.name; + if ( + fontName.Contains(firstMarker, StringComparison.OrdinalIgnoreCase) + && ( + secondMarker == null + || fontName.Contains(secondMarker, StringComparison.OrdinalIgnoreCase) + ) + ) + { + return fontName; + } + } + + return null; } private static bool TrySetupDefaultFont(TerminalUI terminal) @@ -447,32 +537,37 @@ private static bool TrySetupDefaultFont(TerminalUI terminal) return false; } - Font defaultFont = terminal._fontPack._fonts.FirstOrDefault(font => - font.name.Contains("SourceCodePro", StringComparison.OrdinalIgnoreCase) - && font.name.Contains("Regular", StringComparison.OrdinalIgnoreCase) - ); - if (defaultFont == null) + List fonts = terminal._fontPack._fonts; + string defaultFontName = FindFontName(fonts, "SourceCodePro", "Regular"); + if (string.IsNullOrWhiteSpace(defaultFontName)) { - defaultFont = terminal._fontPack._fonts.FirstOrDefault(font => - font.name.Contains("Mono", StringComparison.OrdinalIgnoreCase) - && font.name.Contains("Regular", StringComparison.OrdinalIgnoreCase) - ); + defaultFontName = FindFontName(fonts, "Mono", "Regular"); } - if (defaultFont == null) + if (string.IsNullOrWhiteSpace(defaultFontName)) { - defaultFont = terminal._fontPack._fonts.FirstOrDefault(font => - font.name.Contains("Mono", StringComparison.OrdinalIgnoreCase) - ); + defaultFontName = FindFontName(fonts, "Mono", null); } - if (defaultFont == null) + if (string.IsNullOrWhiteSpace(defaultFontName)) { - defaultFont = terminal._fontPack._fonts.FirstOrDefault(font => - font.name.Contains("Regular", StringComparison.OrdinalIgnoreCase) - ); + defaultFontName = FindFontName(fonts, "Regular", null); } - if (defaultFont == null) + + Font defaultFont; + if (string.IsNullOrWhiteSpace(defaultFontName)) { - defaultFont = terminal._fontPack._fonts.FirstOrDefault(); + defaultFont = fonts[0]; + } + else + { + defaultFont = null; + foreach (Font font in fonts) + { + if (font.name == defaultFontName) + { + defaultFont = font; + break; + } + } } terminal.SetFont(defaultFont, persist: true); @@ -494,7 +589,7 @@ public override void OnInspectorGUI() if (_allCommands.Count == 0 || _defaultCommands.Count == 0) { - HydrateCommandCaches(); + RefreshCommandCaches(); } TerminalUI terminal = target as TerminalUI; @@ -545,26 +640,7 @@ public override void OnInspectorGUI() private void OnEnable() { - _allCommands.Clear(); - _allCommands.UnionWith( - CommandShell - .RegisteredCommands.Value.Select(tuple => tuple.attribute) - .Select(attribute => attribute.Name) - ); - _defaultCommands.Clear(); - _defaultCommands.UnionWith( - CommandShell - .RegisteredCommands.Value.Select(tuple => tuple.attribute) - .Where(tuple => tuple.Default) - .Select(attribute => attribute.Name) - ); - _nonDefaultCommands.Clear(); - _nonDefaultCommands.UnionWith( - CommandShell - .RegisteredCommands.Value.Select(tuple => tuple.attribute) - .Where(tuple => !tuple.Default) - .Select(attribute => attribute.Name) - ); + RefreshCommandCaches(); _fontsByPrefix.Clear(); ResetStateIdempotent(force: true); @@ -598,13 +674,13 @@ private void ResetStateIdempotent(bool force) } TerminalAssetPackPostProcessor.NewFontPacks.Clear(); - if (!_fontPacks.Any()) + if (_fontPacks.Count == 0) { _fontPacks.Clear(); _fontPacks.AddRange(LoadAll()); } - if (!_themePacks.Any()) + if (_themePacks.Count == 0) { _themePacks.Clear(); _themePacks.AddRange(LoadAll()); @@ -685,7 +761,7 @@ private Font GetCurrentlySelectedFont(TerminalUI terminal) try { - return _fontsByPrefix.ToArray()[_fontKey].Value.ToArray()[_secondFontKey].Value; + return FontByKeys(_fontKey, _secondFontKey); } catch { @@ -693,28 +769,46 @@ private Font GetCurrentlySelectedFont(TerminalUI terminal) } } - private void HydrateCommandCaches() + private Font FontByKeys(int fontKey, int secondFontKey) + { + List fontKeyNames = new(_fontsByPrefix.Count); + foreach (string fontKeyName in _fontsByPrefix.Keys) + { + fontKeyNames.Add(fontKeyName); + } + + SortedDictionary availableFonts = _fontsByPrefix[fontKeyNames[fontKey]]; + List secondFontKeyNames = new(availableFonts.Count); + foreach (string secondFontKeyName in availableFonts.Keys) + { + secondFontKeyNames.Add(secondFontKeyName); + } + + return availableFonts[secondFontKeyNames[secondFontKey]]; + } + + private void RefreshCommandCaches() { _allCommands.Clear(); - _allCommands.UnionWith( - CommandShell - .RegisteredCommands.Value.Select(tuple => tuple.attribute) - .Select(attribute => attribute.Name) - ); _defaultCommands.Clear(); - _defaultCommands.UnionWith( - CommandShell - .RegisteredCommands.Value.Select(tuple => tuple.attribute) - .Where(tuple => tuple.Default) - .Select(attribute => attribute.Name) - ); _nonDefaultCommands.Clear(); - _nonDefaultCommands.UnionWith( - CommandShell - .RegisteredCommands.Value.Select(tuple => tuple.attribute) - .Where(tuple => !tuple.Default) - .Select(attribute => attribute.Name) - ); + foreach ( + (MethodInfo method, RegisterCommandAttribute attribute) in CommandShell + .RegisteredCommands + .Value + ) + { + string commandName = attribute.Name; + _allCommands.Add(commandName); + if (attribute.Default) + { + _defaultCommands.Add(commandName); + } + else + { + _nonDefaultCommands.Add(commandName); + } + } } private void RenderCyclingPreviews() @@ -957,7 +1051,7 @@ private bool CheckForThemingAndFontChanges(TerminalUI terminal) EditorGUILayout.BeginHorizontal(); try { - if (!_themePacks.Any()) + if (_themePacks.Count == 0) { GUILayout.Label("NO THEME PACKS", _impactLabelStyle); } @@ -975,7 +1069,7 @@ private bool CheckForThemingAndFontChanges(TerminalUI terminal) _themePackIndex = EditorGUILayout.Popup( _themePackIndex, - _themePacks.Select(themePack => themePack.name).ToArray() + PackNames(_themePacks) ); if (0 <= _themePackIndex && _themePackIndex < _themePacks.Count) { @@ -1009,7 +1103,7 @@ private bool CheckForThemingAndFontChanges(TerminalUI terminal) EditorGUILayout.BeginHorizontal(); try { - if (!_fontPacks.Any()) + if (_fontPacks.Count == 0) { GUILayout.Label("NO FONT PACKS", _impactLabelStyle); } @@ -1025,10 +1119,7 @@ private bool CheckForThemingAndFontChanges(TerminalUI terminal) GUILayout.Label("Select Font Pack:"); } - _fontPackIndex = EditorGUILayout.Popup( - _fontPackIndex, - _fontPacks.Select(fontPack => fontPack.name).ToArray() - ); + _fontPackIndex = EditorGUILayout.Popup(_fontPackIndex, PackNames(_fontPacks)); if (0 <= _fontPackIndex && _fontPackIndex < _fontPacks.Count) { TerminalFontPack fontPack = _fontPacks[_fontPackIndex]; @@ -1082,21 +1173,7 @@ private bool CheckForThemingAndFontChanges(TerminalUI terminal) _themeIndex = EditorGUILayout.Popup( _themeIndex, - terminal - ._themePack._themeNames.Select(theme => - theme - .Replace( - "-theme", - string.Empty, - StringComparison.OrdinalIgnoreCase - ) - .Replace( - "theme-", - string.Empty, - StringComparison.OrdinalIgnoreCase - ) - ) - .ToArray() + FriendlyThemeNames(terminal._themePack._themeNames) ); if (0 <= _themeIndex && _themeIndex < terminal._themePack._themeNames.Count) @@ -1182,7 +1259,8 @@ private bool CheckForIgnoredCommandUpdates(TerminalUI terminal) if (0 < _intermediateResults.Count) { - string[] ignorableCommands = _intermediateResults.ToArray(); + string[] ignorableCommands = new string[_intermediateResults.Count]; + _intermediateResults.CopyTo(ignorableCommands, 0); EditorGUILayout.BeginHorizontal(); try @@ -1289,32 +1367,42 @@ private bool RenderSelectableFonts(TerminalUI terminal) GUILayout.Label("Select Font:"); } - string[] fontKeys = _fontsByPrefix.Keys.ToArray(); - _fontKey = EditorGUILayout.Popup(_fontKey, fontKeys); + List fontKeys = new(_fontsByPrefix.Count); + foreach (string fontKey in _fontsByPrefix.Keys) + { + fontKeys.Add(fontKey); + } + + _fontKey = EditorGUILayout.Popup(_fontKey, fontKeys.ToArray()); if (currentFontKey != _fontKey) { _secondFontKey = -1; } - if (0 <= _fontKey && _fontKey < fontKeys.Length) + if (0 <= _fontKey && _fontKey < fontKeys.Count) { string selectedFontKey = fontKeys[_fontKey]; SortedDictionary availableFonts = _fontsByPrefix[ selectedFontKey ]; - string[] secondFontKeys = availableFonts.Keys.ToArray(); + List secondFontKeys = new(availableFonts.Count); + foreach (string secondFontKey in availableFonts.Keys) + { + secondFontKeys.Add(secondFontKey); + } + Font selectedFont = null; - switch (secondFontKeys.Length) + switch (secondFontKeys.Count) { case > 1: { _secondFontKey = EditorGUILayout.Popup( _secondFontKey, - secondFontKeys + secondFontKeys.ToArray() ); - if (0 <= _secondFontKey && _secondFontKey < secondFontKeys.Length) + if (0 <= _secondFontKey && _secondFontKey < secondFontKeys.Count) { selectedFont = availableFonts[secondFontKeys[_secondFontKey]]; } @@ -1323,7 +1411,12 @@ private bool RenderSelectableFonts(TerminalUI terminal) } case 1: { - selectedFont = availableFonts.Values.Single(); + selectedFont = null; + foreach (Font font in availableFonts.Values) + { + selectedFont = font; + break; + } break; } } diff --git a/Editor/Helper/TerminalThemeStyleSheetHelper.cs b/Editor/Helper/TerminalThemeStyleSheetHelper.cs index eecfb6fb..bbc52936 100644 --- a/Editor/Helper/TerminalThemeStyleSheetHelper.cs +++ b/Editor/Helper/TerminalThemeStyleSheetHelper.cs @@ -4,7 +4,6 @@ namespace WallstopStudios.DxCommandTerminal.Editor.Helper using System; using System.Collections.Generic; using System.IO; - using System.Linq; using System.Text.RegularExpressions; using Themes; using UnityEditor; @@ -143,7 +142,9 @@ public static string[] GetAvailableThemes(StyleSheet styleSheetAsset) lastIndex = nextBraceIndex < 0 ? ussContent.Length : nextBraceIndex + 1; } - return selectors.ToArray(); + string[] result = new string[selectors.Count]; + selectors.CopyTo(result, 0); + return result; } catch (Exception e) { diff --git a/Runtime/CommandTerminal/Backend/BuiltinCommands.cs b/Runtime/CommandTerminal/Backend/BuiltinCommands.cs index e78ab8ce..ac28acc9 100644 --- a/Runtime/CommandTerminal/Backend/BuiltinCommands.cs +++ b/Runtime/CommandTerminal/Backend/BuiltinCommands.cs @@ -3,7 +3,6 @@ namespace WallstopStudios.DxCommandTerminal.Backend using System; using System.Collections.Generic; using System.Diagnostics; - using System.Linq; using System.Text; using Attributes; using Themes; @@ -38,11 +37,19 @@ public static void CommandListThemes(CommandArg[] args) return; } - string themes = string.Join( - BulkSeparator, - terminal._themePack._themeNames.Select(ThemeNameHelper.GetFriendlyThemeName) - ); - Terminal.Log(TerminalLogType.Message, themes); + List themeNames = terminal._themePack._themeNames; + StringBuilder themes = new StringBuilder(); + for (int index = 0; index < themeNames.Count; ++index) + { + if (0 < index) + { + themes.Append(BulkSeparator); + } + + themes.Append(ThemeNameHelper.GetFriendlyThemeName(themeNames[index])); + } + + Terminal.Log(TerminalLogType.Message, themes.ToString()); } [RegisterCommand( @@ -66,11 +73,19 @@ public static void CommandListFonts(CommandArg[] args) return; } - string themes = string.Join( - BulkSeparator, - terminal._fontPack._fonts.Select(font => font.name) - ); - Terminal.Log(TerminalLogType.Message, themes); + List fonts = terminal._fontPack._fonts; + StringBuilder fontNames = new StringBuilder(); + for (int index = 0; index < fonts.Count; ++index) + { + if (0 < index) + { + fontNames.Append(BulkSeparator); + } + + fontNames.Append(fonts[index].name); + } + + Terminal.Log(TerminalLogType.Message, fontNames.ToString()); } [RegisterCommand( @@ -465,7 +480,13 @@ public static void CommandClearAllVariable(CommandArg[] args) } int variableCount = shell.Variables.Count; - foreach (string variable in shell.Variables.Keys.ToArray()) + List variableNames = new List(variableCount); + foreach (string variable in shell.Variables.Keys) + { + variableNames.Add(variable); + } + + foreach (string variable in variableNames) { shell.ClearVariable(variable); } @@ -566,7 +587,7 @@ public static void CommandGetAllVariables(CommandArg[] args) return; } - if (!shell.Variables.Any()) + if (shell.Variables.Count == 0) { Terminal.Log(TerminalLogType.Warning, "No variables found."); return; diff --git a/Runtime/CommandTerminal/Backend/CommandArg.cs b/Runtime/CommandTerminal/Backend/CommandArg.cs index ba25e975..ff321ca9 100644 --- a/Runtime/CommandTerminal/Backend/CommandArg.cs +++ b/Runtime/CommandTerminal/Backend/CommandArg.cs @@ -2,7 +2,6 @@ { using System; using System.Collections.Generic; - using System.Linq; using System.Net; using System.Numerics; using System.Reflection; @@ -41,11 +40,21 @@ public readonly struct CommandArg ">", }; private static readonly Lazy TryGetMethod = new(() => - typeof(CommandArg) - .GetMethods(BindingFlags.Instance | BindingFlags.Public) - .Where(method => method.Name == nameof(TryGet)) - .FirstOrDefault(method => method.GetParameters().Length == 1) - ); + { + foreach ( + MethodInfo method in typeof(CommandArg).GetMethods( + BindingFlags.Instance | BindingFlags.Public + ) + ) + { + if (method.Name == nameof(TryGet) && method.GetParameters().Length == 1) + { + return method; + } + } + + return null; + }); private static readonly Dictionary RegisteredParsers = new(); private static readonly Dictionary< @@ -100,15 +109,15 @@ public string CleanedContents get { string cleanedString = contents; - cleanedString = IgnoredValuesForCleanedTypes.Aggregate( - cleanedString, - (current, ignoredValue) => - current.Replace( - ignoredValue, - string.Empty, - StringComparison.OrdinalIgnoreCase - ) - ); + foreach (string ignoredValue in IgnoredValuesForCleanedTypes) + { + cleanedString = cleanedString.Replace( + ignoredValue, + string.Empty, + StringComparison.OrdinalIgnoreCase + ); + } + return cleanedString; } } @@ -173,25 +182,35 @@ public static int UnregisterAllParsers() private static Dictionary LoadStaticPropertiesForType() { Type type = typeof(T); - return type.GetProperties(BindingFlags.Static | BindingFlags.Public) - .Where(property => property.PropertyType == type) - .ToDictionary( - property => property.Name, - property => property, - StringComparer.OrdinalIgnoreCase - ); + Dictionary properties = new(StringComparer.OrdinalIgnoreCase); + foreach ( + PropertyInfo property in type.GetProperties( + BindingFlags.Static | BindingFlags.Public + ) + ) + { + if (property.PropertyType == type) + { + properties.Add(property.Name, property); + } + } + + return properties; } private static Dictionary LoadStaticFieldsForType() { Type type = typeof(T); - return type.GetFields(BindingFlags.Static | BindingFlags.Public) - .Where(field => field.FieldType == type) - .ToDictionary( - field => field.Name, - field => field, - StringComparer.OrdinalIgnoreCase - ); + Dictionary fields = new(StringComparer.OrdinalIgnoreCase); + foreach (FieldInfo field in type.GetFields(BindingFlags.Static | BindingFlags.Public)) + { + if (field.FieldType == type) + { + fields.Add(field.Name, field); + } + } + + return fields; } private static bool TryGetNamedConstant(string input, out T value) @@ -295,7 +314,12 @@ public bool TryGet(out T parsed, CommandArgParser parserOverride) { if (!EnumValues.TryGetValue(type, out object enumValues)) { - enumValues = Enum.GetValues(type).OfType().ToArray(); + /* + Enum.GetValues returns an array whose runtime type is + exactly T[], so the cast is free; OfType/ToArray would + copy it for nothing. + */ + enumValues = (T[])Enum.GetValues(type); EnumValues[type] = enumValues; } diff --git a/Runtime/CommandTerminal/Backend/CommandArgParsers.cs b/Runtime/CommandTerminal/Backend/CommandArgParsers.cs index 6f317b69..a755a162 100644 --- a/Runtime/CommandTerminal/Backend/CommandArgParsers.cs +++ b/Runtime/CommandTerminal/Backend/CommandArgParsers.cs @@ -2,7 +2,6 @@ namespace WallstopStudios.DxCommandTerminal.Backend { using System; using System.Globalization; - using System.Linq; /// /// Culture-invariant parsers for the built-in types @@ -622,13 +621,18 @@ private static bool TrySplitComponents(string input, out string[] split) return false; } - string strippedInput = CommandArg - .IgnoredValuesForComplexTypes.Where(ignored => !string.IsNullOrEmpty(ignored)) - .Aggregate( - input, - (current, ignored) => - current.Replace(ignored, string.Empty, StringComparison.OrdinalIgnoreCase) - ); + string strippedInput = input; + foreach (string ignored in CommandArg.IgnoredValuesForComplexTypes) + { + if (!string.IsNullOrEmpty(ignored)) + { + strippedInput = strippedInput.Replace( + ignored, + string.Empty, + StringComparison.OrdinalIgnoreCase + ); + } + } foreach (char delimiter in CommandArg.Delimiters) { diff --git a/Runtime/CommandTerminal/Backend/CommandAutoComplete.cs b/Runtime/CommandTerminal/Backend/CommandAutoComplete.cs index bb54e1bd..744d1521 100644 --- a/Runtime/CommandTerminal/Backend/CommandAutoComplete.cs +++ b/Runtime/CommandTerminal/Backend/CommandAutoComplete.cs @@ -2,7 +2,6 @@ namespace WallstopStudios.DxCommandTerminal.Backend { using System; using System.Collections.Generic; - using System.Linq; using Extensions; public sealed class CommandAutoComplete @@ -10,6 +9,7 @@ public sealed class CommandAutoComplete private readonly SortedSet _knownWords = new(StringComparer.OrdinalIgnoreCase); private readonly HashSet _duplicateBuffer = new(StringComparer.OrdinalIgnoreCase); private readonly List _buffer = new(); + private readonly List _historyBuffer = new(); private readonly CommandHistory _history; private readonly CommandShell _shell; @@ -22,12 +22,15 @@ public CommandAutoComplete( { _history = history ?? throw new ArgumentNullException(nameof(history)); _shell = shell ?? throw new ArgumentNullException(nameof(shell)); - _knownWords.UnionWith(commands ?? Enumerable.Empty()); + _knownWords.UnionWith(commands ?? Array.Empty()); } public string[] Complete(string text) { - return Complete(text: text, buffer: _buffer).ToArray(); + Complete(text: text, buffer: _buffer); + string[] results = new string[_buffer.Count]; + _buffer.CopyTo(results); + return results; } public List Complete(string text, List buffer) @@ -49,28 +52,38 @@ List buffer } _duplicateBuffer.Clear(); buffer.Clear(); - foreach ( - string known in _shell - .Commands.Keys.Select(command => - command.NeedsLowerInvariantConversion() - ? command.ToLowerInvariant() - : command - ) - .Concat(_knownWords) - .Concat( - _history.GetHistory(onlySuccess: onlySuccess, onlyErrorFree: onlyErrorFree) - ) - ) + + foreach (string command in _shell.Commands.Keys) + { + TryAddCompletion( + command.NeedsLowerInvariantConversion() ? command.ToLowerInvariant() : command, + input, + buffer + ); + } + + foreach (string known in _knownWords) + { + TryAddCompletion(known, input, buffer); + } + + _history.CopyHistory(onlySuccess, onlyErrorFree, _historyBuffer); + foreach (string entry in _historyBuffer) { - if (!known.StartsWith(input, StringComparison.OrdinalIgnoreCase)) - { - continue; - } + TryAddCompletion(entry, input, buffer); + } + } - if (_duplicateBuffer.Add(known)) - { - buffer.Add(known); - } + private void TryAddCompletion(string candidate, string input, List buffer) + { + if (!candidate.StartsWith(input, StringComparison.OrdinalIgnoreCase)) + { + return; + } + + if (_duplicateBuffer.Add(candidate)) + { + buffer.Add(candidate); } } } diff --git a/Runtime/CommandTerminal/Backend/CommandHistory.cs b/Runtime/CommandTerminal/Backend/CommandHistory.cs index 7925a6d3..1c748401 100644 --- a/Runtime/CommandTerminal/Backend/CommandHistory.cs +++ b/Runtime/CommandTerminal/Backend/CommandHistory.cs @@ -2,7 +2,6 @@ namespace WallstopStudios.DxCommandTerminal.Backend { using System; using System.Collections.Generic; - using System.Linq; using DataStructures; public sealed class CommandHistory @@ -33,10 +32,21 @@ public CommandHistory(int capacity) public IEnumerable GetHistory(bool onlySuccess, bool onlyErrorFree) { - return _history - .Where(value => !onlySuccess || value.success == true) - .Where(value => !onlyErrorFree || value.errorFree == true) - .Select(value => value.text); + for (int index = 0; index < _history.Count; ++index) + { + (string text, bool? success, bool? errorFree) entry = _history[index]; + if (onlySuccess && entry.success != true) + { + continue; + } + + if (onlyErrorFree && entry.errorFree != true) + { + continue; + } + + yield return entry.text; + } } public void Resize(int newCapacity) @@ -129,5 +139,30 @@ public int Clear() _direction = 0; return count; } + + /* + Fills a caller-owned buffer without allocating: the completion + hot path iterates history on every keystroke-driven query, so it + must not pay for enumerators or iterator state machines. + */ + internal void CopyHistory(bool onlySuccess, bool onlyErrorFree, List results) + { + results.Clear(); + for (int index = 0; index < _history.Count; ++index) + { + (string text, bool? success, bool? errorFree) entry = _history[index]; + if (onlySuccess && entry.success != true) + { + continue; + } + + if (onlyErrorFree && entry.errorFree != true) + { + continue; + } + + results.Add(entry.text); + } + } } } diff --git a/Runtime/CommandTerminal/Backend/CommandLog.cs b/Runtime/CommandTerminal/Backend/CommandLog.cs index 54ff725c..dcb7f020 100644 --- a/Runtime/CommandTerminal/Backend/CommandLog.cs +++ b/Runtime/CommandTerminal/Backend/CommandLog.cs @@ -2,7 +2,6 @@ namespace WallstopStudios.DxCommandTerminal.Backend { using System; using System.Collections.Generic; - using System.Linq; using DataStructures; using UnityEngine; @@ -25,7 +24,7 @@ public CommandLog(int maxItems, IEnumerable ignoredLogTypes = n { _logs = new CyclicBuffer(maxItems); this.ignoredLogTypes = new HashSet( - ignoredLogTypes ?? Enumerable.Empty() + ignoredLogTypes ?? Array.Empty() ); } diff --git a/Runtime/CommandTerminal/Backend/CommandShell.cs b/Runtime/CommandTerminal/Backend/CommandShell.cs index 032dc5e4..eb30f2c0 100644 --- a/Runtime/CommandTerminal/Backend/CommandShell.cs +++ b/Runtime/CommandTerminal/Backend/CommandShell.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; using System.Diagnostics; - using System.Linq; using System.Reflection; using System.Runtime.CompilerServices; using System.Text; @@ -614,7 +613,7 @@ public void InitializeAutoRegisteredCommands( IgnoringDefaultCommands = ignoreDefaultCommands; ClearAutoRegisteredCommands(); _ignoredCommands.Clear(); - _ignoredCommands.UnionWith(ignoredCommands ?? Enumerable.Empty()); + _ignoredCommands.UnionWith(ignoredCommands ?? Array.Empty()); foreach (string ignoredCommand in _ignoredCommands) { _commands.Remove(ignoredCommand); @@ -1190,10 +1189,23 @@ User commands win over auto ones (built-ins included). The foreach (KeyValuePair command in _rejectedCommands) { + ParameterInfo[] parameters = command.Value.GetParameters(); + StringBuilder found = new StringBuilder(command.Value.Name).Append('('); + for (int index = 0; index < parameters.Length; ++index) + { + if (0 < index) + { + found.Append(','); + } + + found.Append(parameters[index].ParameterType.Name); + } + + found.Append(')'); IssueErrorMessage( $"{command.Key} has an invalid signature. " + $"Expected: {command.Value.Name}(CommandArg[]). " - + $"Found: {command.Value.Name}({string.Join(",", command.Value.GetParameters().Select(p => p.ParameterType.Name))})" + + $"Found: {found}" ); } diff --git a/Runtime/CommandTerminal/Input/TerminalKeyboardController.cs b/Runtime/CommandTerminal/Input/TerminalKeyboardController.cs index 014ad089..3b1efb7f 100644 --- a/Runtime/CommandTerminal/Input/TerminalKeyboardController.cs +++ b/Runtime/CommandTerminal/Input/TerminalKeyboardController.cs @@ -2,21 +2,13 @@ { using System; using System.Collections.Generic; - using System.Linq; using UI; using UnityEngine; [DisallowMultipleComponent] public class TerminalKeyboardController : MonoBehaviour, IInputHandler { - protected static readonly TerminalControlTypes[] ControlTypes = Enum.GetValues( - typeof(TerminalControlTypes) - ) - .OfType() -#pragma warning disable CS0612 // Type or member is obsolete - .Except(new[] { TerminalControlTypes.None }) -#pragma warning restore CS0612 // Type or member is obsolete - .ToArray(); + protected static readonly TerminalControlTypes[] ControlTypes = BuildControlTypes(); public bool ShouldHandleInputThisFrame { @@ -112,6 +104,22 @@ public TerminalKeyboardController() _controlHandlerActions[TerminalControlTypes.CompleteForward] = Complete; } + private static TerminalControlTypes[] BuildControlTypes() + { + Array values = Enum.GetValues(typeof(TerminalControlTypes)); + List controlTypes = new(values.Length); +#pragma warning disable CS0612 // Type or member is obsolete + foreach (TerminalControlTypes value in values) + { + if (value != TerminalControlTypes.None) + { + controlTypes.Add(value); + } + } +#pragma warning restore CS0612 // Type or member is obsolete + return controlTypes.ToArray(); + } + protected virtual void Awake() { if (terminal != null) @@ -307,8 +315,17 @@ protected virtual bool IsEnterCommandPressed() private void VerifyControlOrderIntegrity() { - TerminalControlTypes[] missingControls = ControlTypes.Except(_controlOrder).ToArray(); - if (0 < missingControls.Length) + List missingControls = null; + foreach (TerminalControlTypes controlType in ControlTypes) + { + if (!_controlOrder.Contains(controlType)) + { + missingControls ??= new List(); + missingControls.Add(controlType); + } + } + + if (missingControls != null) { Debug.LogWarning( $"Control Order is missing the following controls: [{string.Join(", ", missingControls)}]. " diff --git a/Runtime/CommandTerminal/UI/TerminalUI.cs b/Runtime/CommandTerminal/UI/TerminalUI.cs index 779e95a7..a193d385 100644 --- a/Runtime/CommandTerminal/UI/TerminalUI.cs +++ b/Runtime/CommandTerminal/UI/TerminalUI.cs @@ -3,7 +3,7 @@ using System; using System.Collections.Generic; using System.ComponentModel; - using System.Linq; + using System.Globalization; using Attributes; using Backend; using Extensions; @@ -26,6 +26,10 @@ public sealed class TerminalUI : MonoBehaviour // Cache log callback to reduce allocations private static readonly Application.LogCallback UnityLogCallback = HandleUnityLog; + private static readonly List EmptyLogTypes = new(); + + private static readonly List EmptyStrings = new(); + // ReSharper disable once MemberCanBePrivate.Global public bool IsClosed => _state != TerminalState.OpenFull @@ -348,13 +352,13 @@ void TrackProperties(string[] properties, List storage) switch (value) { case List stringList: - value = stringList.ToList(); + value = new List(stringList); break; case List logTypeList: - value = logTypeList.ToList(); + value = new List(logTypeList); break; case List fontList: - value = fontList.ToList(); + value = new List(fontList); break; } _propertyValues[property.name] = value; @@ -454,16 +458,10 @@ private void RefreshStaticState(bool force) { Terminal.Buffer.Resize(logBufferSize); } - if ( - !Terminal.Buffer.ignoredLogTypes.SetEquals( - _ignoredLogTypes ?? Enumerable.Empty() - ) - ) + if (!Terminal.Buffer.ignoredLogTypes.SetEquals(_ignoredLogTypes ?? EmptyLogTypes)) { Terminal.Buffer.ignoredLogTypes.Clear(); - Terminal.Buffer.ignoredLogTypes.UnionWith( - _ignoredLogTypes ?? Enumerable.Empty() - ); + Terminal.Buffer.ignoredLogTypes.UnionWith(_ignoredLogTypes ?? EmptyLogTypes); } } @@ -490,9 +488,7 @@ private void RefreshStaticState(bool force) if ( Terminal.Shell.IgnoringDefaultCommands != ignoreDefaultCommands || !Terminal.Shell.AutoCommandsRegistered - || !Terminal.Shell.IgnoredCommands.SetEquals( - _disabledCommands ?? Enumerable.Empty() - ) + || !Terminal.Shell.IgnoredCommands.SetEquals(_disabledCommands ?? EmptyStrings) ) { Terminal.Shell.ClearAutoRegisteredCommands(); @@ -597,10 +593,10 @@ propertyValue is List currentStringList && previousValue is List previousStringList ) { - if (!currentStringList.SequenceEqual(previousStringList)) + if (!ListsEqual(currentStringList, previousStringList)) { needRefresh = true; - _propertyValues[property.name] = currentStringList.ToList(); + _propertyValues[property.name] = new List(currentStringList); } continue; @@ -610,10 +606,12 @@ propertyValue is List currentLogTypeList && previousValue is List previousLogTypeList ) { - if (!currentLogTypeList.SequenceEqual(previousLogTypeList)) + if (!ListsEqual(currentLogTypeList, previousLogTypeList)) { needRefresh = true; - _propertyValues[property.name] = currentLogTypeList.ToList(); + _propertyValues[property.name] = new List( + currentLogTypeList + ); } continue; @@ -836,6 +834,76 @@ private static void HandleUnityLog(string message, string stackTrace, LogType ty Terminal.Buffer?.HandleLog(message, stackTrace, (TerminalLogType)type); } + private static bool NamesContain(List names, string candidate) + { + foreach (string name in names) + { + if (string.Equals(name, candidate, StringComparison.OrdinalIgnoreCase)) + { + return true; + } + } + + return false; + } + + private static string FindName(List names, string marker) + { + foreach (string name in names) + { + if (name.Contains(marker, StringComparison.OrdinalIgnoreCase)) + { + return name; + } + } + + return null; + } + + private static Font FindFont(List fonts, bool requireMono, bool requireRegular) + { + foreach (Font font in fonts) + { + string fontName = font.name; + if (!requireMono || fontName.Contains("Mono", StringComparison.OrdinalIgnoreCase)) + { + if ( + !requireRegular + || fontName.Contains("Regular", StringComparison.OrdinalIgnoreCase) + ) + { + return font; + } + } + } + + return null; + } + + private static bool ListsEqual(List left, List right) + { + if (ReferenceEquals(left, right)) + { + return true; + } + + if (left.Count != right.Count) + { + return false; + } + + EqualityComparer comparer = EqualityComparer.Default; + for (int index = 0; index < left.Count; ++index) + { + if (!comparer.Equals(left[index], right[index])) + { + return false; + } + } + + return true; + } + public void ToggleState(TerminalState newState) { SetState(_state == newState ? TerminalState.Closed : newState); @@ -1019,7 +1087,7 @@ bool IsValidTheme(out string validTheme) } List themeNames = _themePack._themeNames; - if (themeNames.Contains(theme, StringComparer.OrdinalIgnoreCase)) + if (NamesContain(themeNames, theme)) { validTheme = theme; return true; @@ -1027,7 +1095,7 @@ bool IsValidTheme(out string validTheme) foreach (string themeName in ThemeNameHelper.GetPossibleThemeNames(theme)) { - if (themeNames.Contains(themeName, StringComparer.OrdinalIgnoreCase)) + if (NamesContain(themeNames, themeName)) { validTheme = themeName; return true; @@ -1063,10 +1131,14 @@ void SetRuntimeTheme() return; } - string[] loadedThemes = terminalRoot - .GetClasses() - .Where(ThemeNameHelper.IsThemeName) - .ToArray(); + List loadedThemes = new(); + foreach (string cssClass in terminalRoot.GetClasses()) + { + if (ThemeNameHelper.IsThemeName(cssClass)) + { + loadedThemes.Add(cssClass); + } + } foreach (string loadedTheme in loadedThemes) { @@ -1754,18 +1826,14 @@ private void InitializeTheme(VisualElement root) if (themeNames is { Count: > 0 }) { - _runtimeTheme = themeNames.FirstOrDefault(theme => - theme.Contains("dark", StringComparison.OrdinalIgnoreCase) - ); + _runtimeTheme = FindName(themeNames, "dark"); if (_runtimeTheme == null) { - _runtimeTheme = themeNames.FirstOrDefault(theme => - theme.Contains("light", StringComparison.OrdinalIgnoreCase) - ); + _runtimeTheme = FindName(themeNames, "light"); } if (_runtimeTheme == null) { - _runtimeTheme = themeNames.FirstOrDefault(); + _runtimeTheme = themeNames[0]; } /* @@ -1803,25 +1871,18 @@ private void InitializeFont() List loadedFonts = _fontPack._fonts; if (loadedFonts is { Count: > 0 }) { - _runtimeFont = loadedFonts.FirstOrDefault(font => - font.name.Contains("Mono", StringComparison.OrdinalIgnoreCase) - && font.name.Contains("Regular", StringComparison.OrdinalIgnoreCase) - ); + _runtimeFont = FindFont(loadedFonts, requireMono: true, requireRegular: true); if (_runtimeFont == null) { - _runtimeFont = loadedFonts.FirstOrDefault(font => - font.name.Contains("Mono", StringComparison.OrdinalIgnoreCase) - ); + _runtimeFont = FindFont(loadedFonts, requireMono: true, requireRegular: false); } if (_runtimeFont == null) { - _runtimeFont = loadedFonts.FirstOrDefault(font => - font.name.Contains("Regular", StringComparison.OrdinalIgnoreCase) - ); + _runtimeFont = FindFont(loadedFonts, requireMono: false, requireRegular: true); } if (_runtimeFont == null) { - _runtimeFont = loadedFonts.FirstOrDefault(); + _runtimeFont = loadedFonts[0]; } } diff --git a/Runtime/DataStructures/CyclicBuffer.cs b/Runtime/DataStructures/CyclicBuffer.cs index 0bc1d268..c96f18d1 100644 --- a/Runtime/DataStructures/CyclicBuffer.cs +++ b/Runtime/DataStructures/CyclicBuffer.cs @@ -3,7 +3,6 @@ namespace WallstopStudios.DxCommandTerminal.DataStructures using System; using System.Collections; using System.Collections.Generic; - using System.Linq; using Extensions; [Serializable] @@ -40,7 +39,7 @@ public CyclicBuffer(int capacity, IEnumerable initialContents = null) _position = 0; Count = 0; _buffer = new List(); - foreach (T item in initialContents ?? Enumerable.Empty()) + foreach (T item in initialContents ?? Array.Empty()) { Add(item); } diff --git a/tooling~/package.json b/tooling~/package.json index e65db6f2..95cbf8dd 100644 --- a/tooling~/package.json +++ b/tooling~/package.json @@ -14,6 +14,7 @@ "lint:member-ordering:fix": "node scripts/lint-member-ordering.mjs --fix", "lint:multiline-comments": "node scripts/lint-multiline-comments.mjs", "lint:multiline-comments:fix": "node scripts/lint-multiline-comments.mjs --fix", + "lint:linq-production": "node scripts/lint-linq-production.mjs", "unity:mcp:probe": "node scripts/mcp/unity-mcp.mjs probe", "unity:mcp:configure": "node scripts/mcp/unity-mcp.mjs configure", "unity:mcp:bridge": "node scripts/mcp/unity-mcp.mjs bridge", diff --git a/tooling~/scripts/lint-linq-production.mjs b/tooling~/scripts/lint-linq-production.mjs new file mode 100644 index 00000000..9335749a --- /dev/null +++ b/tooling~/scripts/lint-linq-production.mjs @@ -0,0 +1,138 @@ +/* + Production code bans LINQ (PR #60 review). + + Every LINQ operator allocates: each call chain materializes at least one + enumerator per stage plus one closure per lambda, and several operators + (`ToArray`, `ToDictionary`, `ToList`, `OfType`) copy the whole sequence. In a + performance-first runtime library that must hit zero-allocation hot paths, LINQ + can only ever be a silent allocation regression, so it is banned outright in + shipped code (`Runtime/`, `Editor/`). Tests and `Generator~` tooling are exempt: + they are not on any runtime path. + + Detection is line-based and intentionally broad: a `using System.Linq` + directive fails even when the file no longer calls any operator, because a + dead directive invites the next call. Fully-qualified `System.Linq.` uses and + static `Enumerable.` calls are caught too, so the rule cannot be dodged by + skipping the directive. Comment-only lines are skipped so documentation about + the ban cannot trip it. + + There is no `--fix` on purpose: removing LINQ means choosing loop shapes and + buffer strategies per call site, which is a code change, not a mechanical + rewrite. + + Exit codes: 0 = clean, 1 = at least one violation (or nothing was scanned). + + Adapted from the repository's lint-multiline-comments.mjs structure + (issues #50/#51 lineage) and unity-helpers' linter conventions + (MIT, Ambiguous-Interactive). +*/ +import fs from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const REPO_ROOT = path.resolve( + path.dirname(fileURLToPath(import.meta.url)), + "../.." +); +// Overridable so the contract tests can point the scan at a fixture tree. Nothing in CI sets it. +const SCAN_ROOTS = process.env.LINQ_PRODUCTION_ROOTS + ? process.env.LINQ_PRODUCTION_ROOTS.split(path.delimiter).filter(Boolean) + : ["Runtime", "Editor"]; + +const USING_PATTERN = /\busing\s+System\s*\.\s*Linq\b/; +const QUALIFIED_PATTERN = /\bSystem\s*\.\s*Linq\s*\./; +const ENUMERABLE_PATTERN = /\bEnumerable\s*\.\s*\w+/; + +function isCommentOnly(line) { + const trimmed = line.trimStart(); + return ( + trimmed.startsWith("//") || + trimmed.startsWith("/*") || + trimmed.startsWith("*") + ); +} + +/** Returns one-based line numbers whose code (not comment) text violates the ban. */ +export function linqViolations(text) { + const lines = text.split("\n"); + const violations = []; + for (let index = 0; index < lines.length; index++) { + const line = lines[index]; + if (isCommentOnly(line)) { + continue; + } + + if ( + USING_PATTERN.test(line) || + QUALIFIED_PATTERN.test(line) || + ENUMERABLE_PATTERN.test(line) + ) { + violations.push({ line: index + 1, text: line.trim() }); + } + } + + return violations; +} + +function listFiles(root) { + const absolute = path.resolve(REPO_ROOT, root); + if (!fs.existsSync(absolute)) { + // A missing root contributes nothing; the caller reports an empty walk as an error + // rather than reading the absence of a scan as green. + return []; + } + const found = []; + const walk = (dir) => { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const child = path.join(dir, entry.name); + if (entry.isDirectory()) { + if (entry.name === "bin" || entry.name === "obj" || entry.name === "node_modules") { + continue; + } + walk(child); + } else if (entry.name.endsWith(".cs")) { + found.push(child); + } + } + }; + walk(absolute); + return found; +} + +function main() { + const files = SCAN_ROOTS.flatMap((root) => listFiles(root)); + if (files.length === 0) { + console.error( + `[linq-production] ERROR: no C# files were found under ${SCAN_ROOTS.join(", ")}, so this run checked nothing. A scan that matched nothing is the absence of a measurement, not a pass.` + ); + return 1; + } + + let violations = 0; + for (const file of files) { + const text = fs.readFileSync(file, "utf8"); + const fileViolations = linqViolations(text); + if (fileViolations.length === 0) { + continue; + } + const relative = path.relative(REPO_ROOT, file); + for (const violation of fileViolations) { + violations++; + console.error( + `${relative}:${violation.line}: LINQ in production code: ${violation.text}` + ); + } + } + + console.log(`[linq-production] ${files.length} file(s) scanned`); + return 0 < violations ? 1 : 0; +} + +if (process.argv[1] === fileURLToPath(import.meta.url)) { + /* + exitCode, not exit: on Windows, writes to a piped stderr are asynchronous, and a + process.exit() would truncate the violation report this process is still flushing. + Letting the loop drain keeps the report whole and the exit code identical. + */ + process.exitCode = main(process.argv.slice(2)); +} diff --git a/tooling~/scripts/tests/lint-linq-production.test.mjs b/tooling~/scripts/tests/lint-linq-production.test.mjs new file mode 100644 index 00000000..40482928 --- /dev/null +++ b/tooling~/scripts/tests/lint-linq-production.test.mjs @@ -0,0 +1,172 @@ +/* + Contract tests for tooling~/scripts/lint-linq-production.mjs. + + The rule: production code (Runtime/, Editor/) bans LINQ outright - the using + directive, fully-qualified System.Linq calls, and static Enumerable calls all + fail, because each shape invites the allocation-heavy operator chain the ban + exists to prevent. The negative cases carry the weight: comment-only lines that + mention the ban's vocabulary, List.ToArray()/CopyTo (instance methods, not + LINQ), Tests and Generator~ directories, and fixture-shaped code must all stay + silent, or a sweep buries real production LINQ in noise. The positive cases pin + every detection shape. +*/ +import test from "node:test"; +import assert from "node:assert"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { spawnSync } from "node:child_process"; +import { fileURLToPath, pathToFileURL } from "node:url"; + +const repoRoot = path.resolve( + path.dirname(fileURLToPath(import.meta.url)), + "../.." +); +const linterPath = path.join(repoRoot, "scripts", "lint-linq-production.mjs"); +const { linqViolations } = await import(pathToFileURL(linterPath).href); + +function violationsIn(source) { + return linqViolations(source); +} + +/** Shapes that mention LINQ vocabulary and must stay silent. */ +const EXEMPT = [ + ["a clean production file", "namespace N\n{\n public sealed class C\n {\n }\n}"], + + [ + "List.ToArray() instance method", + "List list = new();\nstring[] copy = list.ToArray();", + ], + + [ + "List.CopyTo instance method", + "string[] result = new string[list.Count];\nlist.CopyTo(result, 0);", + ], + + [ + "Dictionary.Keys.CopyTo instance method", + "string[] keys = new string[map.Count];\nmap.Keys.CopyTo(keys, 0);", + ], + + [ + "a comment-only line mentioning the ban", + "// using System.Linq is banned in production code", + ], + + [ + "a comment-only line mentioning Enumerable", + "/* Enumerable.Empty would allocate here, so use Array.Empty */", + ], + + [ + "a doc comment mentioning LINQ", + "/// Does not use LINQ.", + ], + + [ + "a trailing comment after clean code", + "string[] copy = list.ToArray(); // copied without LINQ", + ], + + [ + "a type whose name merely contains Enumerable", + "sealed class EnumerableFactory\n{\n void Make() { }\n}", + ], + + [ + "an identifier containing the word linq", + "int linqFreeCount = 0;", + ], +]; + +/** Shapes that violate the ban. Every one is caught. */ +const VIOLATIONS = [ + ["the using directive", "using System.Linq;"], + ["the indented using directive", " using System.Linq;"], + ["the qualified static call", "System.Linq.Enumerable.Empty()"], + ["the static Enumerable call", "Enumerable.Range(0, 10)"], + ["the static Enumerable call with a space", "Enumerable .Empty()"], + ["the using with odd spacing", "using System.Linq;"], +]; + +test("exempt shapes stay silent", () => { + for (const [name, source] of EXEMPT) { + assert.deepStrictEqual(violationsIn(source), [], name); + } +}); + +test("violation shapes are caught", () => { + for (const [name, source] of VIOLATIONS) { + assert.strictEqual(violationsIn(source).length, 1, name); + } +}); + +test("line numbers are one-based and count every violating line", () => { + const source = [ + "namespace N", + "{", + " using System.Linq;", + " int[] values = Enumerable.Range(0, 3).ToArray();", + "}", + ].join("\n"); + const violations = violationsIn(source); + assert.deepStrictEqual( + violations.map((violation) => violation.line), + [3, 4] + ); +}); + +function runLinter(fixtureRoot) { + return spawnSync(process.execPath, [linterPath], { + cwd: repoRoot, + env: { ...process.env, LINQ_PRODUCTION_ROOTS: fixtureRoot }, + encoding: "utf8", + }); +} + +test("a fixture tree with production LINQ fails with a report", () => { + const fixtureRoot = fs.mkdtempSync(path.join(os.tmpdir(), "linq-lint-")); + const production = path.join(fixtureRoot, "Production"); + fs.mkdirSync(production, { recursive: true }); + fs.writeFileSync( + path.join(production, "Bad.cs"), + "using System.Linq;\npublic sealed class C { int[] V() => Enumerable.Range(0, 2).ToArray(); }\n" + ); + try { + const result = runLinter(fixtureRoot); + assert.strictEqual(result.status, 1, result.stderr); + assert.ok(result.stderr.includes("Bad.cs:1"), result.stderr); + assert.ok(result.stderr.includes("Bad.cs:2"), result.stderr); + assert.ok(result.stdout.includes("1 file(s) scanned"), result.stdout); + } finally { + fs.rmSync(fixtureRoot, { recursive: true, force: true }); + } +}); + +test("a fixture tree with clean production code passes", () => { + const fixtureRoot = fs.mkdtempSync(path.join(os.tmpdir(), "linq-lint-")); + const production = path.join(fixtureRoot, "Production"); + fs.mkdirSync(production, { recursive: true }); + fs.writeFileSync( + path.join(production, "Clean.cs"), + "List list = new();\nstring[] copy = list.ToArray();\n// using System.Linq is banned here\n" + ); + try { + const result = runLinter(fixtureRoot); + assert.strictEqual(result.status, 0, result.stderr); + assert.ok(result.stdout.includes("1 file(s) scanned"), result.stdout); + } finally { + fs.rmSync(fixtureRoot, { recursive: true, force: true }); + } +}); + +test("a missing root fails loudly instead of scanning nothing", () => { + const fixtureRoot = fs.mkdtempSync(path.join(os.tmpdir(), "linq-lint-")); + try { + const result = runLinter(fixtureRoot); + assert.strictEqual(result.status, 1, result.stderr); + assert.ok(result.stderr.includes("checked nothing"), result.stderr); + } finally { + fs.rmSync(fixtureRoot, { recursive: true, force: true }); + } +}); From 499a917db9e1e555e44dcba7424bdb527a06aa3b Mon Sep 17 00:00:00 2001 From: wallstop Date: Fri, 11 Sep 2026 20:59:17 +0000 Subject: [PATCH 5/8] Cache builders and derived data on the LINQ-free paths Reviewer follow-up on 102bd87: removing LINQ must not re-introduce the allocations through loops. - New CachedStringBuilder (ThreadStatic rent/return, capacity retained); theme/font listings and the shell rejected-signature message rent instead of allocating per call. - TerminalUIEditor popup arrays and font-key arrays are cached against their sources (reference + count stamps) and rebuilt only on change; no per-frame ToArray() in OnGUI. - Bulk variable clear moved into CommandShell.ClearVariables with a cached snapshot buffer. - foreach over List with first/last separators instead of counting loops (context.md rule 11); Counting loops kept only where the index is genuinely used. - context.md rule 23 codifies the allocation discipline. PlayMode 199/199 on Unity 6000.4.6f1; tooling 127 pass; linters green. --- .llm/context.md | 15 ++ Editor/CustomEditors/TerminalUIEditor.cs | 233 ++++++++++++------ .../Backend/BuiltinCommands.cs | 67 +++-- .../CommandTerminal/Backend/CommandHistory.cs | 9 +- .../CommandTerminal/Backend/CommandShell.cs | 59 ++++- Runtime/Helper/CachedStringBuilder.cs | 41 +++ Runtime/Helper/CachedStringBuilder.cs.meta | 3 + 7 files changed, 306 insertions(+), 121 deletions(-) create mode 100644 Runtime/Helper/CachedStringBuilder.cs create mode 100644 Runtime/Helper/CachedStringBuilder.cs.meta diff --git a/.llm/context.md b/.llm/context.md index 3f14a590..e8c0486e 100644 --- a/.llm/context.md +++ b/.llm/context.md @@ -163,6 +163,21 @@ frontmatter validity, index freshness, and pointer-file delegation; see intermediate sequences. `List.ToArray()`/`CopyTo` instance methods stay legal. Tests and `Generator~` tooling are exempt. Enforced by `npm --prefix tooling~ run lint:linq-production` (pre-commit + CI; no `:fix` by design). +23. Replacing LINQ is not enough - the loop must not re-introduce the allocation. Rules + that came out of the PR #60 allocation review: + - String assembly on repeated paths rents a builder: + `CachedStringBuilder.Rent(capacity)` / `Return(builder)` in `Runtime/Helper/` + (ThreadStatic, capacity retained). Never `new StringBuilder()` per call. + - Derived data drawn every `OnGUI`/editor tick is cached against its source + (reference + count stamp) and rebuilt only when the source changes - e.g. the + popup option arrays and font-key arrays in `TerminalUIEditor`. A fresh array or + list per frame is a regression even when the loop itself is allocation-free. + - Snapshot-then-mutate patterns (clear all variables while iterating a dictionary) + live on the owning type with a cached buffer field (`CommandShell.ClearVariables`), + not in command handlers that build throwaway lists. + - When converting LINQ, enumerate with `foreach` over the concrete type (struct + enumerator, bounds-check elision); keep counting loops only where the index is + genuinely used, per rule 11. ### Unity Package Rules diff --git a/Editor/CustomEditors/TerminalUIEditor.cs b/Editor/CustomEditors/TerminalUIEditor.cs index 52a2853b..1e3e72b8 100644 --- a/Editor/CustomEditors/TerminalUIEditor.cs +++ b/Editor/CustomEditors/TerminalUIEditor.cs @@ -26,6 +26,16 @@ namespace WallstopStudios.DxCommandTerminal.Editor.CustomEditors public sealed class TerminalUIEditor : Editor { private static readonly TimeSpan CycleInterval = TimeSpan.FromSeconds(0.75); + private static List _themePackNamesSource; + private static string[] _themePackNames; + private static List _fontPackNamesSource; + private static string[] _fontPackNames; + private static List _themeDisplayNameSource; + private static string[] _themeDisplayNames; + private static SortedDictionary> _fontKeySource; + private static string[] _fontKeyCache = Array.Empty(); + private static SortedDictionary _secondFontKeySource; + private static string[] _secondFontKeyCache = Array.Empty(); private int _commandIndex; private TerminalUI _lastSeen; @@ -46,6 +56,13 @@ public sealed class TerminalUIEditor : Editor private readonly List _fontPacks = new(); private readonly List _themePacks = new(); + /* + OnGUI redraws every frame, so the popup option arrays and font-key + arrays are cached and rebuilt only when their source changes + (reference + count stamps). Static because the derived data is a + pure function of the shared pack lists; every inspector instance + renders the same options. + */ private int _themeIndex = -1; private int _fontKey = -1; private int _secondFontKey = -1; @@ -394,6 +411,19 @@ SortedDictionary> fontsByPrefix } } + private static bool NamesContain(List names, string candidate) + { + foreach (string name in names) + { + if (string.Equals(name, candidate, StringComparison.OrdinalIgnoreCase)) + { + return true; + } + } + + return false; + } + private static bool TrySetupDefaultTheme(TerminalUI terminal) { if ( @@ -416,54 +446,6 @@ private static bool TrySetupDefaultTheme(TerminalUI terminal) return true; } - private static string[] PackNames(List themePacks) - { - string[] names = new string[themePacks.Count]; - for (int index = 0; index < themePacks.Count; ++index) - { - names[index] = themePacks[index].name; - } - - return names; - } - - private static string[] PackNames(List fontPacks) - { - string[] names = new string[fontPacks.Count]; - for (int index = 0; index < fontPacks.Count; ++index) - { - names[index] = fontPacks[index].name; - } - - return names; - } - - private static string[] FriendlyThemeNames(List themeNames) - { - string[] displayNames = new string[themeNames.Count]; - for (int index = 0; index < themeNames.Count; ++index) - { - displayNames[index] = themeNames[index] - .Replace("-theme", string.Empty, StringComparison.OrdinalIgnoreCase) - .Replace("theme-", string.Empty, StringComparison.OrdinalIgnoreCase); - } - - return displayNames; - } - - private static bool NamesContain(List names, string candidate) - { - foreach (string name in names) - { - if (string.Equals(name, candidate, StringComparison.OrdinalIgnoreCase)) - { - return true; - } - } - - return false; - } - private static string FindThemeName( List names, string firstMarker, @@ -574,6 +556,119 @@ private static bool TrySetupDefaultFont(TerminalUI terminal) return true; } + private static void RefreshFontKeyCaches( + SortedDictionary> fontsByPrefix + ) + { + if ( + ReferenceEquals(_fontKeySource, fontsByPrefix) + && _fontKeyCache.Length == fontsByPrefix.Count + ) + { + return; + } + + string[] keys = new string[fontsByPrefix.Count]; + int position = 0; + foreach (string fontKeyName in fontsByPrefix.Keys) + { + keys[position++] = fontKeyName; + } + + _fontKeySource = fontsByPrefix; + _fontKeyCache = keys; + } + + private static string[] RefreshSecondFontKeys(SortedDictionary availableFonts) + { + if ( + ReferenceEquals(_secondFontKeySource, availableFonts) + && _secondFontKeyCache.Length == availableFonts.Count + ) + { + return _secondFontKeyCache; + } + + string[] keys = new string[availableFonts.Count]; + int position = 0; + foreach (string secondFontKeyName in availableFonts.Keys) + { + keys[position++] = secondFontKeyName; + } + + _secondFontKeySource = availableFonts; + _secondFontKeyCache = keys; + return keys; + } + + private static string[] FriendlyThemeNames(List themeNames) + { + if ( + _themeDisplayNames == null + || !ReferenceEquals(_themeDisplayNameSource, themeNames) + || _themeDisplayNames.Length != themeNames.Count + ) + { + string[] displayNames = new string[themeNames.Count]; + int position = 0; + foreach (string themeName in themeNames) + { + displayNames[position++] = themeName + .Replace("-theme", string.Empty, StringComparison.OrdinalIgnoreCase) + .Replace("theme-", string.Empty, StringComparison.OrdinalIgnoreCase); + } + + _themeDisplayNameSource = themeNames; + _themeDisplayNames = displayNames; + } + + return _themeDisplayNames; + } + + private static string[] PackNames(List themePacks) + { + if ( + _themePackNames == null + || !ReferenceEquals(_themePackNamesSource, themePacks) + || _themePackNames.Length != themePacks.Count + ) + { + string[] names = new string[themePacks.Count]; + int position = 0; + foreach (TerminalThemePack themePack in themePacks) + { + names[position++] = themePack.name; + } + + _themePackNamesSource = themePacks; + _themePackNames = names; + } + + return _themePackNames; + } + + private static string[] PackNames(List fontPacks) + { + if ( + _fontPackNames == null + || !ReferenceEquals(_fontPackNamesSource, fontPacks) + || _fontPackNames.Length != fontPacks.Count + ) + { + string[] names = new string[fontPacks.Count]; + int position = 0; + foreach (TerminalFontPack fontPack in fontPacks) + { + names[position++] = fontPack.name; + } + + _fontPackNamesSource = fontPacks; + _fontPackNames = names; + } + + return _fontPackNames; + } + public override void OnInspectorGUI() { _impactButtonStyle ??= new GUIStyle(GUI.skin.button) @@ -771,20 +866,9 @@ private Font GetCurrentlySelectedFont(TerminalUI terminal) private Font FontByKeys(int fontKey, int secondFontKey) { - List fontKeyNames = new(_fontsByPrefix.Count); - foreach (string fontKeyName in _fontsByPrefix.Keys) - { - fontKeyNames.Add(fontKeyName); - } - - SortedDictionary availableFonts = _fontsByPrefix[fontKeyNames[fontKey]]; - List secondFontKeyNames = new(availableFonts.Count); - foreach (string secondFontKeyName in availableFonts.Keys) - { - secondFontKeyNames.Add(secondFontKeyName); - } - - return availableFonts[secondFontKeyNames[secondFontKey]]; + RefreshFontKeyCaches(_fontsByPrefix); + SortedDictionary availableFonts = _fontsByPrefix[_fontKeyCache[fontKey]]; + return availableFonts[RefreshSecondFontKeys(availableFonts)[secondFontKey]]; } private void RefreshCommandCaches() @@ -1367,42 +1451,33 @@ private bool RenderSelectableFonts(TerminalUI terminal) GUILayout.Label("Select Font:"); } - List fontKeys = new(_fontsByPrefix.Count); - foreach (string fontKey in _fontsByPrefix.Keys) - { - fontKeys.Add(fontKey); - } - - _fontKey = EditorGUILayout.Popup(_fontKey, fontKeys.ToArray()); + RefreshFontKeyCaches(_fontsByPrefix); + _fontKey = EditorGUILayout.Popup(_fontKey, _fontKeyCache); if (currentFontKey != _fontKey) { _secondFontKey = -1; } - if (0 <= _fontKey && _fontKey < fontKeys.Count) + if (0 <= _fontKey && _fontKey < _fontKeyCache.Length) { - string selectedFontKey = fontKeys[_fontKey]; + string selectedFontKey = _fontKeyCache[_fontKey]; SortedDictionary availableFonts = _fontsByPrefix[ selectedFontKey ]; - List secondFontKeys = new(availableFonts.Count); - foreach (string secondFontKey in availableFonts.Keys) - { - secondFontKeys.Add(secondFontKey); - } + string[] secondFontKeys = RefreshSecondFontKeys(availableFonts); Font selectedFont = null; - switch (secondFontKeys.Count) + switch (secondFontKeys.Length) { case > 1: { _secondFontKey = EditorGUILayout.Popup( _secondFontKey, - secondFontKeys.ToArray() + secondFontKeys ); - if (0 <= _secondFontKey && _secondFontKey < secondFontKeys.Count) + if (0 <= _secondFontKey && _secondFontKey < secondFontKeys.Length) { selectedFont = availableFonts[secondFontKeys[_secondFontKey]]; } diff --git a/Runtime/CommandTerminal/Backend/BuiltinCommands.cs b/Runtime/CommandTerminal/Backend/BuiltinCommands.cs index ac28acc9..3f97fd6a 100644 --- a/Runtime/CommandTerminal/Backend/BuiltinCommands.cs +++ b/Runtime/CommandTerminal/Backend/BuiltinCommands.cs @@ -5,6 +5,7 @@ namespace WallstopStudios.DxCommandTerminal.Backend using System.Diagnostics; using System.Text; using Attributes; + using Helper; using Themes; using UI; using UnityEngine; @@ -14,6 +15,10 @@ public static class BuiltInCommands { private const string BulkSeparator = " "; + private const int AverageThemeNameCapacity = 24; + + private const int AverageFontNameCapacity = 32; + private static readonly StringBuilder StringBuilder = new(); [RegisterCommand( @@ -38,18 +43,29 @@ public static void CommandListThemes(CommandArg[] args) } List themeNames = terminal._themePack._themeNames; - StringBuilder themes = new StringBuilder(); - for (int index = 0; index < themeNames.Count; ++index) + StringBuilder themes = CachedStringBuilder.Rent( + themeNames.Count * AverageThemeNameCapacity + ); + try { - if (0 < index) + bool first = true; + foreach (string themeName in themeNames) { - themes.Append(BulkSeparator); + if (!first) + { + themes.Append(BulkSeparator); + } + + themes.Append(ThemeNameHelper.GetFriendlyThemeName(themeName)); + first = false; } - themes.Append(ThemeNameHelper.GetFriendlyThemeName(themeNames[index])); + Terminal.Log(TerminalLogType.Message, themes.ToString()); + } + finally + { + CachedStringBuilder.Return(themes); } - - Terminal.Log(TerminalLogType.Message, themes.ToString()); } [RegisterCommand( @@ -74,18 +90,29 @@ public static void CommandListFonts(CommandArg[] args) } List fonts = terminal._fontPack._fonts; - StringBuilder fontNames = new StringBuilder(); - for (int index = 0; index < fonts.Count; ++index) + StringBuilder fontNames = CachedStringBuilder.Rent( + fonts.Count * AverageFontNameCapacity + ); + try { - if (0 < index) + bool first = true; + foreach (Font font in fonts) { - fontNames.Append(BulkSeparator); + if (!first) + { + fontNames.Append(BulkSeparator); + } + + fontNames.Append(font.name); + first = false; } - fontNames.Append(fonts[index].name); + Terminal.Log(TerminalLogType.Message, fontNames.ToString()); + } + finally + { + CachedStringBuilder.Return(fontNames); } - - Terminal.Log(TerminalLogType.Message, fontNames.ToString()); } [RegisterCommand( @@ -479,17 +506,7 @@ public static void CommandClearAllVariable(CommandArg[] args) return; } - int variableCount = shell.Variables.Count; - List variableNames = new List(variableCount); - foreach (string variable in shell.Variables.Keys) - { - variableNames.Add(variable); - } - - foreach (string variable in variableNames) - { - shell.ClearVariable(variable); - } + int variableCount = shell.ClearVariables(); Terminal.Log( variableCount == 0 diff --git a/Runtime/CommandTerminal/Backend/CommandHistory.cs b/Runtime/CommandTerminal/Backend/CommandHistory.cs index 1c748401..6d19c869 100644 --- a/Runtime/CommandTerminal/Backend/CommandHistory.cs +++ b/Runtime/CommandTerminal/Backend/CommandHistory.cs @@ -32,9 +32,8 @@ public CommandHistory(int capacity) public IEnumerable GetHistory(bool onlySuccess, bool onlyErrorFree) { - for (int index = 0; index < _history.Count; ++index) + foreach ((string text, bool? success, bool? errorFree) entry in _history) { - (string text, bool? success, bool? errorFree) entry = _history[index]; if (onlySuccess && entry.success != true) { continue; @@ -143,14 +142,14 @@ public int Clear() /* Fills a caller-owned buffer without allocating: the completion hot path iterates history on every keystroke-driven query, so it - must not pay for enumerators or iterator state machines. + must not pay for enumerators or iterator state machines. The + buffer's struct enumerator keeps this loop allocation-free. */ internal void CopyHistory(bool onlySuccess, bool onlyErrorFree, List results) { results.Clear(); - for (int index = 0; index < _history.Count; ++index) + foreach ((string text, bool? success, bool? errorFree) entry in _history) { - (string text, bool? success, bool? errorFree) entry = _history[index]; if (onlySuccess && entry.success != true) { continue; diff --git a/Runtime/CommandTerminal/Backend/CommandShell.cs b/Runtime/CommandTerminal/Backend/CommandShell.cs index eb30f2c0..cb79e965 100644 --- a/Runtime/CommandTerminal/Backend/CommandShell.cs +++ b/Runtime/CommandTerminal/Backend/CommandShell.cs @@ -9,6 +9,7 @@ using System.Threading; using Attributes; using DataStructures; + using Helper; using UnityEngine; using Debug = UnityEngine.Debug; @@ -130,6 +131,8 @@ like the rest of the shell. StringComparer.OrdinalIgnoreCase ); + private readonly List _variableClearBuffer = new(); + /* Depth-scoped parse buffers: one per active RunCommand/TryComplete level, so nested dispatches cannot corrupt outer arguments. @@ -1023,6 +1026,28 @@ public bool ClearVariable(string name) return _variables.Remove(name); } + /* + Clears every variable in one call. The snapshot list is cached so + repeated bulk clears (or a clear run from inside a command while + the shell is iterating) never allocate; dictionary keys cannot be + enumerated while entries are being removed. + */ + public int ClearVariables() + { + _variableClearBuffer.Clear(); + foreach (string variable in _variables.Keys) + { + _variableClearBuffer.Add(variable); + } + + foreach (string variable in _variableClearBuffer) + { + _variables.Remove(variable); + } + + return _variableClearBuffer.Count; + } + // ReSharper disable once MemberCanBePrivate.Global public bool SetVariable(string name, CommandArg value) { @@ -1190,23 +1215,33 @@ User commands win over auto ones (built-ins included). The foreach (KeyValuePair command in _rejectedCommands) { ParameterInfo[] parameters = command.Value.GetParameters(); - StringBuilder found = new StringBuilder(command.Value.Name).Append('('); - for (int index = 0; index < parameters.Length; ++index) + StringBuilder found = CachedStringBuilder.Rent(64); + try { - if (0 < index) + found.Append(command.Value.Name).Append('('); + bool first = true; + foreach (ParameterInfo parameter in parameters) { - found.Append(','); + if (!first) + { + found.Append(','); + } + + found.Append(parameter.ParameterType.Name); + first = false; } - found.Append(parameters[index].ParameterType.Name); + found.Append(')'); + IssueErrorMessage( + $"{command.Key} has an invalid signature. " + + $"Expected: {command.Value.Name}(CommandArg[]). " + + $"Found: {found}" + ); + } + finally + { + CachedStringBuilder.Return(found); } - - found.Append(')'); - IssueErrorMessage( - $"{command.Key} has an invalid signature. " - + $"Expected: {command.Value.Name}(CommandArg[]). " - + $"Found: {found}" - ); } AutoCommandsRegistered = true; diff --git a/Runtime/Helper/CachedStringBuilder.cs b/Runtime/Helper/CachedStringBuilder.cs new file mode 100644 index 00000000..7e050829 --- /dev/null +++ b/Runtime/Helper/CachedStringBuilder.cs @@ -0,0 +1,41 @@ +namespace WallstopStudios.DxCommandTerminal.Helper +{ + using System; + using System.Text; + using UnityEngine; + + /// + /// Rents one per thread so repeated string + /// assembly (log listings, diagnostics) does not allocate on every call. + /// Rent before appending and Return in a finally block; the returned + /// builder keeps its capacity and is cleared on return. + /// + internal static class CachedStringBuilder + { + /* + A static builder shared across calls would race between threads; + ThreadStatic keeps each thread's buffer private. + */ + [ThreadStatic] + private static StringBuilder _cached; + + public static StringBuilder Rent(int minimumCapacity) + { + StringBuilder builder = _cached; + _cached = null; + if (builder == null) + { + return new StringBuilder(Mathf.Max(minimumCapacity, 16)); + } + + builder.EnsureCapacity(minimumCapacity); + return builder; + } + + public static void Return(StringBuilder builder) + { + builder.Clear(); + _cached = builder; + } + } +} diff --git a/Runtime/Helper/CachedStringBuilder.cs.meta b/Runtime/Helper/CachedStringBuilder.cs.meta new file mode 100644 index 00000000..4a9ffa41 --- /dev/null +++ b/Runtime/Helper/CachedStringBuilder.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: bdc777ef82de4120668b9af4ad42f151 +timeCreated: 1744903000 \ No newline at end of file From 8f669947ed3aecbf654efbc4da13834d5bdcd6d4 Mon Sep 17 00:00:00 2001 From: wallstop Date: Fri, 11 Sep 2026 21:38:10 +0000 Subject: [PATCH 6/8] Reduce reflection, consolidate editor caches, fix linter comment handling Review round on 499a917, fixes for 14 points: - CommandArg.TryGet(Type, out object): untyped parser adapters replace MakeGenericMethod+Invoke - the non-generic path is reflection-free at dispatch (IL2CPP/WebGL safe); registered parsers mirror into the same delegate shape at registration time. - TerminalKeyboardController.ControlTypes: written out explicitly, no Enum.GetValues at runtime; the all-members test guards drift. - CommandShell: rejected signatures store their formatted text at rejection time, so the error pass never reflects; ClearVariables is capture-count-and-clear. - TerminalUIEditor: one RefreshCache builder for PackNames/FriendlyThemeNames; one RefreshKeyCache builder using ICollection.CopyTo (bulk); stale flags set where _fontsByPrefix is rebuilt (Bugbot: same reference plus same count after an in-place refill kept stale keys); FriendlyThemeName only allocates when a marker is present; RegisteredCommands iterated via var so System.Reflection leaves the file. - CachedStringBuilder.Scope: amortized zero-allocation struct for using statements; replaces try/finally at the rental sites. - lint-linq-production.mjs masks comments and string literals with the comparison-direction scanner, so banned vocabulary in block comments or strings cannot trip it (Bugbot); 5 new contract cases. PlayMode 199/199 on Unity 6000.4.6f1; generator 30/30; payload byte-compare OK; tooling 132 pass. --- Editor/CustomEditors/TerminalUIEditor.cs | 203 +++++----- .../Backend/BuiltinCommands.cs | 52 +-- Runtime/CommandTerminal/Backend/CommandArg.cs | 351 ++++++++++++++++-- .../CommandTerminal/Backend/CommandShell.cs | 60 ++- .../Input/TerminalKeyboardController.cs | 25 +- Runtime/Helper/CachedStringBuilder.cs | 29 +- tooling~/scripts/lint-linq-production.mjs | 73 +++- .../tests/lint-linq-production.test.mjs | 20 + 8 files changed, 590 insertions(+), 223 deletions(-) diff --git a/Editor/CustomEditors/TerminalUIEditor.cs b/Editor/CustomEditors/TerminalUIEditor.cs index 1e3e72b8..4dc08f98 100644 --- a/Editor/CustomEditors/TerminalUIEditor.cs +++ b/Editor/CustomEditors/TerminalUIEditor.cs @@ -5,7 +5,6 @@ namespace WallstopStudios.DxCommandTerminal.Editor.CustomEditors using System.Collections.Generic; using System.Diagnostics; using System.IO; - using System.Reflection; using Attributes; using Backend; using DxCommandTerminal.Helper; @@ -32,9 +31,9 @@ public sealed class TerminalUIEditor : Editor private static string[] _fontPackNames; private static List _themeDisplayNameSource; private static string[] _themeDisplayNames; - private static SortedDictionary> _fontKeySource; + private static bool _fontKeyCacheStale = true; private static string[] _fontKeyCache = Array.Empty(); - private static SortedDictionary _secondFontKeySource; + private static bool _secondFontKeyCacheStale = true; private static string[] _secondFontKeyCache = Array.Empty(); private int _commandIndex; @@ -556,117 +555,109 @@ private static bool TrySetupDefaultFont(TerminalUI terminal) return true; } - private static void RefreshFontKeyCaches( - SortedDictionary> fontsByPrefix + /* + One cache builder for every derived name array: rebuilds only when + the source list changes (reference + count stamps) and otherwise + hands back the cached array, so OnGUI never allocates per frame. + */ + private static string[] RefreshCache( + List source, + Func nameOf, + ref List cachedSource, + ref string[] cached ) { if ( - ReferenceEquals(_fontKeySource, fontsByPrefix) - && _fontKeyCache.Length == fontsByPrefix.Count + cached == null + || !ReferenceEquals(cachedSource, source) + || cached.Length != source.Count ) { - return; - } + string[] names = new string[source.Count]; + int position = 0; + foreach (T item in source) + { + names[position++] = nameOf(item); + } - string[] keys = new string[fontsByPrefix.Count]; - int position = 0; - foreach (string fontKeyName in fontsByPrefix.Keys) - { - keys[position++] = fontKeyName; + cachedSource = source; + cached = names; } - _fontKeySource = fontsByPrefix; - _fontKeyCache = keys; + return cached; } - private static string[] RefreshSecondFontKeys(SortedDictionary availableFonts) + private static string[] PackNames(List themePacks) { - if ( - ReferenceEquals(_secondFontKeySource, availableFonts) - && _secondFontKeyCache.Length == availableFonts.Count - ) - { - return _secondFontKeyCache; - } - - string[] keys = new string[availableFonts.Count]; - int position = 0; - foreach (string secondFontKeyName in availableFonts.Keys) - { - keys[position++] = secondFontKeyName; - } + return RefreshCache( + themePacks, + static pack => pack.name, + ref _themePackNamesSource, + ref _themePackNames + ); + } - _secondFontKeySource = availableFonts; - _secondFontKeyCache = keys; - return keys; + private static string[] PackNames(List fontPacks) + { + return RefreshCache( + fontPacks, + static pack => pack.name, + ref _fontPackNamesSource, + ref _fontPackNames + ); } private static string[] FriendlyThemeNames(List themeNames) { - if ( - _themeDisplayNames == null - || !ReferenceEquals(_themeDisplayNameSource, themeNames) - || _themeDisplayNames.Length != themeNames.Count - ) - { - string[] displayNames = new string[themeNames.Count]; - int position = 0; - foreach (string themeName in themeNames) - { - displayNames[position++] = themeName - .Replace("-theme", string.Empty, StringComparison.OrdinalIgnoreCase) - .Replace("theme-", string.Empty, StringComparison.OrdinalIgnoreCase); - } - - _themeDisplayNameSource = themeNames; - _themeDisplayNames = displayNames; - } - - return _themeDisplayNames; + return RefreshCache( + themeNames, + static name => name, + ref _themeDisplayNameSource, + ref _themeDisplayNames + ); } - private static string[] PackNames(List themePacks) + /* + Strips the "-theme"/"theme-" markers only when present, so clean + names never allocate a replacement string. + */ + private static string FriendlyThemeName(string themeName) { if ( - _themePackNames == null - || !ReferenceEquals(_themePackNamesSource, themePacks) - || _themePackNames.Length != themePacks.Count + themeName.Contains("-theme", StringComparison.OrdinalIgnoreCase) + || themeName.Contains("theme-", StringComparison.OrdinalIgnoreCase) ) { - string[] names = new string[themePacks.Count]; - int position = 0; - foreach (TerminalThemePack themePack in themePacks) - { - names[position++] = themePack.name; - } - - _themePackNamesSource = themePacks; - _themePackNames = names; + return themeName + .Replace("-theme", string.Empty, StringComparison.OrdinalIgnoreCase) + .Replace("theme-", string.Empty, StringComparison.OrdinalIgnoreCase); } - return _themePackNames; + return themeName; } - private static string[] PackNames(List fontPacks) + /* + One key-array builder for both popups: `keys` is a live collection + view, so the caller decides when its contents may have changed + (force) and otherwise the cache holds on reference + count. + Rebuilds use ICollection.CopyTo, the bulk operation. + */ + private static string[] RefreshKeyCache( + ICollection keys, + bool force, + ref bool stale, + ref string[] cached + ) { - if ( - _fontPackNames == null - || !ReferenceEquals(_fontPackNamesSource, fontPacks) - || _fontPackNames.Length != fontPacks.Count - ) + if (!force && !stale && cached.Length == keys.Count) { - string[] names = new string[fontPacks.Count]; - int position = 0; - foreach (TerminalFontPack fontPack in fontPacks) - { - names[position++] = fontPack.name; - } - - _fontPackNamesSource = fontPacks; - _fontPackNames = names; + return cached; } - return _fontPackNames; + string[] rebuilt = new string[keys.Count]; + keys.CopyTo(rebuilt, 0); + stale = false; + return cached = rebuilt; } public override void OnInspectorGUI() @@ -733,10 +724,32 @@ public override void OnInspectorGUI() } } + private string[] FontKeys() + { + return RefreshKeyCache( + _fontsByPrefix.Keys, + force: false, + ref _fontKeyCacheStale, + ref _fontKeyCache + ); + } + + private string[] SecondFontKeys(SortedDictionary availableFonts) + { + return RefreshKeyCache( + availableFonts.Keys, + force: _fontKeyCacheStale, + ref _secondFontKeyCacheStale, + ref _secondFontKeyCache + ); + } + private void OnEnable() { RefreshCommandCaches(); _fontsByPrefix.Clear(); + _fontKeyCacheStale = true; + _secondFontKeyCacheStale = true; ResetStateIdempotent(force: true); @@ -788,6 +801,8 @@ private void ResetStateIdempotent(bool force) } _fontsByPrefix.Clear(); + _fontKeyCacheStale = true; + _secondFontKeyCacheStale = true; CollectFonts(terminal, _fontsByPrefix); _persistThemeChanges = false; @@ -866,9 +881,9 @@ private Font GetCurrentlySelectedFont(TerminalUI terminal) private Font FontByKeys(int fontKey, int secondFontKey) { - RefreshFontKeyCaches(_fontsByPrefix); - SortedDictionary availableFonts = _fontsByPrefix[_fontKeyCache[fontKey]]; - return availableFonts[RefreshSecondFontKeys(availableFonts)[secondFontKey]]; + string[] fontKeys = FontKeys(); + SortedDictionary availableFonts = _fontsByPrefix[fontKeys[fontKey]]; + return availableFonts[SecondFontKeys(availableFonts)[secondFontKey]]; } private void RefreshCommandCaches() @@ -876,11 +891,7 @@ private void RefreshCommandCaches() _allCommands.Clear(); _defaultCommands.Clear(); _nonDefaultCommands.Clear(); - foreach ( - (MethodInfo method, RegisterCommandAttribute attribute) in CommandShell - .RegisteredCommands - .Value - ) + foreach (var (method, attribute) in CommandShell.RegisteredCommands.Value) { string commandName = attribute.Name; _allCommands.Add(commandName); @@ -1451,8 +1462,8 @@ private bool RenderSelectableFonts(TerminalUI terminal) GUILayout.Label("Select Font:"); } - RefreshFontKeyCaches(_fontsByPrefix); - _fontKey = EditorGUILayout.Popup(_fontKey, _fontKeyCache); + string[] fontKeys = FontKeys(); + _fontKey = EditorGUILayout.Popup(_fontKey, fontKeys); if (currentFontKey != _fontKey) { @@ -1461,11 +1472,11 @@ private bool RenderSelectableFonts(TerminalUI terminal) if (0 <= _fontKey && _fontKey < _fontKeyCache.Length) { - string selectedFontKey = _fontKeyCache[_fontKey]; + string selectedFontKey = fontKeys[_fontKey]; SortedDictionary availableFonts = _fontsByPrefix[ selectedFontKey ]; - string[] secondFontKeys = RefreshSecondFontKeys(availableFonts); + string[] secondFontKeys = SecondFontKeys(availableFonts); Font selectedFont = null; switch (secondFontKeys.Length) diff --git a/Runtime/CommandTerminal/Backend/BuiltinCommands.cs b/Runtime/CommandTerminal/Backend/BuiltinCommands.cs index 3f97fd6a..c38304d3 100644 --- a/Runtime/CommandTerminal/Backend/BuiltinCommands.cs +++ b/Runtime/CommandTerminal/Backend/BuiltinCommands.cs @@ -43,29 +43,22 @@ public static void CommandListThemes(CommandArg[] args) } List themeNames = terminal._themePack._themeNames; - StringBuilder themes = CachedStringBuilder.Rent( + using CachedStringBuilder.Scope themes = new( themeNames.Count * AverageThemeNameCapacity ); - try + bool firstTheme = true; + foreach (string themeName in themeNames) { - bool first = true; - foreach (string themeName in themeNames) + if (!firstTheme) { - if (!first) - { - themes.Append(BulkSeparator); - } - - themes.Append(ThemeNameHelper.GetFriendlyThemeName(themeName)); - first = false; + themes.Builder.Append(BulkSeparator); } - Terminal.Log(TerminalLogType.Message, themes.ToString()); - } - finally - { - CachedStringBuilder.Return(themes); + themes.Builder.Append(ThemeNameHelper.GetFriendlyThemeName(themeName)); + firstTheme = false; } + + Terminal.Log(TerminalLogType.Message, themes.Builder.ToString()); } [RegisterCommand( @@ -90,29 +83,20 @@ public static void CommandListFonts(CommandArg[] args) } List fonts = terminal._fontPack._fonts; - StringBuilder fontNames = CachedStringBuilder.Rent( - fonts.Count * AverageFontNameCapacity - ); - try + using CachedStringBuilder.Scope fontNames = new(fonts.Count * AverageFontNameCapacity); + bool firstFont = true; + foreach (Font font in fonts) { - bool first = true; - foreach (Font font in fonts) + if (!firstFont) { - if (!first) - { - fontNames.Append(BulkSeparator); - } - - fontNames.Append(font.name); - first = false; + fontNames.Builder.Append(BulkSeparator); } - Terminal.Log(TerminalLogType.Message, fontNames.ToString()); - } - finally - { - CachedStringBuilder.Return(fontNames); + fontNames.Builder.Append(font.name); + firstFont = false; } + + Terminal.Log(TerminalLogType.Message, fontNames.Builder.ToString()); } [RegisterCommand( diff --git a/Runtime/CommandTerminal/Backend/CommandArg.cs b/Runtime/CommandTerminal/Backend/CommandArg.cs index ff321ca9..cbbc6a16 100644 --- a/Runtime/CommandTerminal/Backend/CommandArg.cs +++ b/Runtime/CommandTerminal/Backend/CommandArg.cs @@ -15,6 +15,12 @@ public readonly struct CommandArg { // Public to allow custom-mutation, if desired + /// + /// Untyped parser adapter: parses into a boxed value for the + /// non-generic path. + /// + internal delegate bool UntypedParser(string input, out object parsed); + public static readonly HashSet Delimiters = new() { ',', ';', ':', '_', '/', '\\' }; public static readonly List Quotes = new() { '"', '\'' }; public static readonly HashSet IgnoredValuesForCleanedTypes = new() { "\r", "\n" }; @@ -39,24 +45,229 @@ public readonly struct CommandArg "<", ">", }; - private static readonly Lazy TryGetMethod = new(() => + private static readonly Dictionary RegisteredParsers = new(); + + /* + Untyped adapters for the built-in parser table, so the non-generic + TryGet(Type, out object) path needs no reflection at runtime + (IL2CPP/WebGL safe). Registered parsers mirror into the same + delegate shape at registration time. + */ + private static readonly Dictionary BuiltInUntypedParsers = new() { - foreach ( - MethodInfo method in typeof(CommandArg).GetMethods( - BindingFlags.Instance | BindingFlags.Public - ) - ) + [typeof(bool)] = (string input, out object parsed) => { - if (method.Name == nameof(TryGet) && method.GetParameters().Length == 1) - { - return method; - } - } - - return null; - }); + bool ok = CommandArgParsers.Bool(input, out bool value); + parsed = value; + return ok; + }, + [typeof(float)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.Float(input, out float value); + parsed = value; + return ok; + }, + [typeof(int)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.Int(input, out int value); + parsed = value; + return ok; + }, + [typeof(uint)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.Uint(input, out uint value); + parsed = value; + return ok; + }, + [typeof(long)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.Long(input, out long value); + parsed = value; + return ok; + }, + [typeof(ulong)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.Ulong(input, out ulong value); + parsed = value; + return ok; + }, + [typeof(double)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.Double(input, out double value); + parsed = value; + return ok; + }, + [typeof(short)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.Short(input, out short value); + parsed = value; + return ok; + }, + [typeof(ushort)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.Ushort(input, out ushort value); + parsed = value; + return ok; + }, + [typeof(byte)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.Byte(input, out byte value); + parsed = value; + return ok; + }, + [typeof(sbyte)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.Sbyte(input, out sbyte value); + parsed = value; + return ok; + }, + [typeof(Guid)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.Guid(input, out Guid value); + parsed = value; + return ok; + }, + [typeof(DateTime)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.DateTime(input, out DateTime value); + parsed = value; + return ok; + }, + [typeof(DateTimeOffset)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.DateTimeOffset(input, out DateTimeOffset value); + parsed = value; + return ok; + }, + [typeof(char)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.Char(input, out char value); + parsed = value; + return ok; + }, + [typeof(decimal)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.Decimal(input, out decimal value); + parsed = value; + return ok; + }, + [typeof(BigInteger)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.BigInteger(input, out BigInteger value); + parsed = value; + return ok; + }, + [typeof(TimeSpan)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.TimeSpan(input, out TimeSpan value); + parsed = value; + return ok; + }, + [typeof(Version)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.Version(input, out Version value); + parsed = value; + return ok; + }, + [typeof(IPAddress)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.IPAddress(input, out IPAddress value); + parsed = value; + return ok; + }, + [typeof(Vector2)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.Vector2(input, out Vector2 value); + parsed = value; + return ok; + }, + [typeof(Vector3)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.Vector3(input, out Vector3 value); + parsed = value; + return ok; + }, + [typeof(Vector4)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.Vector4(input, out Vector4 value); + parsed = value; + return ok; + }, + [typeof(Vector2Int)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.Vector2Int(input, out Vector2Int value); + parsed = value; + return ok; + }, + [typeof(Vector3Int)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.Vector3Int(input, out Vector3Int value); + parsed = value; + return ok; + }, + [typeof(Color)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.Color(input, out Color value); + parsed = value; + return ok; + }, + [typeof(Quaternion)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.Quaternion(input, out Quaternion value); + parsed = value; + return ok; + }, + [typeof(Rect)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.Rect(input, out Rect value); + parsed = value; + return ok; + }, + [typeof(RectInt)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.RectInt(input, out RectInt value); + parsed = value; + return ok; + }, + [typeof(Bounds)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.Bounds(input, out Bounds value); + parsed = value; + return ok; + }, + [typeof(BoundsInt)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.BoundsInt(input, out BoundsInt value); + parsed = value; + return ok; + }, + [typeof(RectOffset)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.RectOffset(input, out RectOffset value); + parsed = value; + return ok; + }, + [typeof(Plane)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.Plane(input, out Plane value); + parsed = value; + return ok; + }, + [typeof(Ray)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.Ray(input, out Ray value); + parsed = value; + return ok; + }, + [typeof(Complex)] = (string input, out object parsed) => + { + bool ok = CommandArgParsers.Complex(input, out Complex value); + parsed = value; + return ok; + }, + }; - private static readonly Dictionary RegisteredParsers = new(); + private static readonly Dictionary RegisteredUntypedParsers = new(); private static readonly Dictionary< Type, Dictionary @@ -141,13 +352,26 @@ public static bool RegisterParser(CommandArgParser parser, bool force = fa } Type type = typeof(T); + UntypedParser untypedParser = (string input, out object parsed) => + { + bool ok = parser(input, out T value); + parsed = value; + return ok; + }; if (force) { RegisteredParsers[type] = parser; + RegisteredUntypedParsers[type] = untypedParser; return true; } - return RegisteredParsers.TryAdd(type, parser); + if (!RegisteredParsers.TryAdd(type, parser)) + { + return false; + } + + RegisteredUntypedParsers[type] = untypedParser; + return true; } public static bool TryGetParser(out CommandArgParser parser) @@ -169,6 +393,7 @@ public static bool UnregisterParser() public static bool UnregisterParser(Type type) { + RegisteredUntypedParsers.Remove(type); return RegisteredParsers.Remove(type); } @@ -176,12 +401,12 @@ public static int UnregisterAllParsers() { int parserCount = RegisteredParsers.Count; RegisteredParsers.Clear(); + RegisteredUntypedParsers.Clear(); return parserCount; } - private static Dictionary LoadStaticPropertiesForType() + private static Dictionary LoadStaticPropertiesForType(Type type) { - Type type = typeof(T); Dictionary properties = new(StringComparer.OrdinalIgnoreCase); foreach ( PropertyInfo property in type.GetProperties( @@ -198,9 +423,8 @@ PropertyInfo property in type.GetProperties( return properties; } - private static Dictionary LoadStaticFieldsForType() + private static Dictionary LoadStaticFieldsForType(Type type) { - Type type = typeof(T); Dictionary fields = new(StringComparer.OrdinalIgnoreCase); foreach (FieldInfo field in type.GetFields(BindingFlags.Static | BindingFlags.Public)) { @@ -213,33 +437,42 @@ private static Dictionary LoadStaticFieldsForType() return fields; } - private static bool TryGetNamedConstant(string input, out T value) + private static bool TryGetNamedConstantUntyped(Type type, string input, out object value) { - Type type = typeof(T); if ( !StaticProperties.TryGetValue(type, out Dictionary properties) ) { - properties = LoadStaticPropertiesForType(); + properties = LoadStaticPropertiesForType(type); StaticProperties[type] = properties; } if (properties.TryGetValue(input, out PropertyInfo property)) { - object resolved = property.GetValue(null); - value = (T)resolved; + value = property.GetValue(null); return true; } if (!ConstFields.TryGetValue(type, out Dictionary fields)) { - fields = LoadStaticFieldsForType(); + fields = LoadStaticFieldsForType(type); ConstFields[type] = fields; } if (fields.TryGetValue(input, out FieldInfo field)) { - object resolved = field.GetValue(null); + value = field.GetValue(null); + return true; + } + + value = default; + return false; + } + + private static bool TryGetNamedConstant(string input, out T value) + { + if (TryGetNamedConstantUntyped(typeof(T), input, out object resolved)) + { value = (T)resolved; return true; } @@ -250,19 +483,65 @@ private static bool TryGetNamedConstant(string input, out T value) public bool TryGet(Type type, out object parsed) { - // TODO: Convert into delegates and cache for performance - MethodInfo genericMethod = TryGetMethod.Value; - if (genericMethod == null) + if (type == null) { parsed = default; return false; } - MethodInfo constructed = genericMethod.MakeGenericMethod(type); - object[] parameters = { null }; - bool success = (bool)constructed.Invoke(this, parameters); - parsed = parameters[0]; - return success; + string stringValue = DoNotCleanTypes.Contains(type) ? contents : CleanedContents; + + if (RegisteredUntypedParsers.TryGetValue(type, out UntypedParser registered)) + { + return registered(stringValue, out parsed); + } + + if (type == typeof(string)) + { + parsed = stringValue; + return true; + } + + if (TryGetNamedConstantUntyped(type, stringValue, out parsed)) + { + return true; + } + + if (BuiltInUntypedParsers.TryGetValue(type, out UntypedParser builtIn)) + { + return builtIn(stringValue, out parsed); + } + + if (type.IsEnum) + { + if (Enum.IsDefined(type, stringValue)) + { + if (Enum.TryParse(type, stringValue, out object parsedObject)) + { + parsed = parsedObject; + return true; + } + } + + if (CommandArgParsers.Int(stringValue, out int enumIntValue)) + { + if (!EnumValues.TryGetValue(type, out object enumValues)) + { + enumValues = Enum.GetValues(type); + EnumValues[type] = enumValues; + } + + Array values = (Array)enumValues; + if (0 <= enumIntValue && enumIntValue < values.Length) + { + parsed = values.GetValue(enumIntValue); + return true; + } + } + } + + parsed = default; + return false; } public bool TryGet(out T parsed) @@ -319,7 +598,7 @@ Enum.GetValues returns an array whose runtime type is exactly T[], so the cast is free; OfType/ToArray would copy it for nothing. */ - enumValues = (T[])Enum.GetValues(type); + enumValues = Enum.GetValues(type); EnumValues[type] = enumValues; } diff --git a/Runtime/CommandTerminal/Backend/CommandShell.cs b/Runtime/CommandTerminal/Backend/CommandShell.cs index cb79e965..1da2750e 100644 --- a/Runtime/CommandTerminal/Backend/CommandShell.cs +++ b/Runtime/CommandTerminal/Backend/CommandShell.cs @@ -123,7 +123,13 @@ like the rest of the shell. private readonly CommandHistory _history; private readonly HashSet _ignoredCommands = new(StringComparer.OrdinalIgnoreCase); - private readonly SortedDictionary _rejectedCommands = new( + /* + Rejected signatures store their formatted "Found: ..." text at + rejection time, so the error pass never reflects over MethodInfo. + Rejections only arise on the reflection compatibility path; the + generator catalog never produces them. + */ + private readonly SortedDictionary _rejectedCommands = new( StringComparer.OrdinalIgnoreCase ); @@ -1212,36 +1218,13 @@ User commands win over auto ones (built-ins included). The StringComparer.OrdinalIgnoreCase ); - foreach (KeyValuePair command in _rejectedCommands) + foreach (KeyValuePair command in _rejectedCommands) { - ParameterInfo[] parameters = command.Value.GetParameters(); - StringBuilder found = CachedStringBuilder.Rent(64); - try - { - found.Append(command.Value.Name).Append('('); - bool first = true; - foreach (ParameterInfo parameter in parameters) - { - if (!first) - { - found.Append(','); - } - - found.Append(parameter.ParameterType.Name); - first = false; - } - - found.Append(')'); - IssueErrorMessage( - $"{command.Key} has an invalid signature. " - + $"Expected: {command.Value.Name}(CommandArg[]). " - + $"Found: {found}" - ); - } - finally - { - CachedStringBuilder.Return(found); - } + IssueErrorMessage( + $"{command.Key} has an invalid signature. " + + $"Expected: {command.Key}(CommandArg[]). " + + $"Found: {command.Value}" + ); } AutoCommandsRegistered = true; @@ -1282,7 +1265,22 @@ private void RegisterRejectedCommand(AutoCommand command, string commandName) return; } - _rejectedCommands.TryAdd(commandName, method); + using CachedStringBuilder.Scope found = new(64); + found.Builder.Append(method.Name).Append('('); + bool first = true; + foreach (ParameterInfo parameter in method.GetParameters()) + { + if (!first) + { + found.Builder.Append(','); + } + + found.Builder.Append(parameter.ParameterType.Name); + first = false; + } + + found.Builder.Append(')'); + _rejectedCommands.TryAdd(commandName, found.Builder.ToString()); } private List GetDispatchScope(int depth) diff --git a/Runtime/CommandTerminal/Input/TerminalKeyboardController.cs b/Runtime/CommandTerminal/Input/TerminalKeyboardController.cs index 3b1efb7f..42a12c79 100644 --- a/Runtime/CommandTerminal/Input/TerminalKeyboardController.cs +++ b/Runtime/CommandTerminal/Input/TerminalKeyboardController.cs @@ -106,18 +106,25 @@ public TerminalKeyboardController() private static TerminalControlTypes[] BuildControlTypes() { - Array values = Enum.GetValues(typeof(TerminalControlTypes)); - List controlTypes = new(values.Length); + /* + Written out explicitly rather than via Enum.GetValues: no + runtime reflection, IL2CPP/WebGL safe. The + ControlTypesContainsAllNonNoneEnumValues test fails when a new + enum member is not added here. + */ #pragma warning disable CS0612 // Type or member is obsolete - foreach (TerminalControlTypes value in values) + return new[] { - if (value != TerminalControlTypes.None) - { - controlTypes.Add(value); - } - } + TerminalControlTypes.Close, + TerminalControlTypes.EnterCommand, + TerminalControlTypes.Previous, + TerminalControlTypes.Next, + TerminalControlTypes.ToggleFull, + TerminalControlTypes.ToggleSmall, + TerminalControlTypes.CompleteForward, + TerminalControlTypes.CompleteBackward, + }; #pragma warning restore CS0612 // Type or member is obsolete - return controlTypes.ToArray(); } protected virtual void Awake() diff --git a/Runtime/Helper/CachedStringBuilder.cs b/Runtime/Helper/CachedStringBuilder.cs index 7e050829..62425922 100644 --- a/Runtime/Helper/CachedStringBuilder.cs +++ b/Runtime/Helper/CachedStringBuilder.cs @@ -7,8 +7,9 @@ namespace WallstopStudios.DxCommandTerminal.Helper /// /// Rents one per thread so repeated string /// assembly (log listings, diagnostics) does not allocate on every call. - /// Rent before appending and Return in a finally block; the returned - /// builder keeps its capacity and is cleared on return. + /// Use through in a using statement, which + /// returns the builder on scope exit without allocating: the scope is a + /// stack-only struct, and the rented builder keeps its capacity. /// internal static class CachedStringBuilder { @@ -37,5 +38,29 @@ public static void Return(StringBuilder builder) builder.Clear(); _cached = builder; } + + /// + /// Amortized zero-allocation scope for one rental: constructs with + /// and returns the builder + /// on dispose. Use in a using statement; never store the + /// scope or its beyond the using block. + /// + public readonly struct Scope : IDisposable + { + public StringBuilder Builder => _builder; + + private readonly StringBuilder _builder; + + public Scope(int minimumCapacity) + { + _builder = Rent(minimumCapacity); + } + + public void Dispose() + { + _builder?.Clear(); + _cached = _builder; + } + } } } diff --git a/tooling~/scripts/lint-linq-production.mjs b/tooling~/scripts/lint-linq-production.mjs index 9335749a..948a7030 100644 --- a/tooling~/scripts/lint-linq-production.mjs +++ b/tooling~/scripts/lint-linq-production.mjs @@ -28,7 +28,7 @@ */ import fs from "node:fs"; import path from "node:path"; -import { fileURLToPath } from "node:url"; +import { fileURLToPath, pathToFileURL } from "node:url"; const REPO_ROOT = path.resolve( path.dirname(fileURLToPath(import.meta.url)), @@ -39,29 +39,72 @@ const SCAN_ROOTS = process.env.LINQ_PRODUCTION_ROOTS ? process.env.LINQ_PRODUCTION_ROOTS.split(path.delimiter).filter(Boolean) : ["Runtime", "Editor"]; +const { consumeLiteral } = await import( + pathToFileURL( + path.join(REPO_ROOT, "tooling~", "scripts", "lint-comparison-direction.mjs") + ).href +); + +/** + * Replaces comment contents with spaces, keeping newlines, so banned + * vocabulary inside comments can never trip the scan. String/char literals + * are consumed with the comparison-direction scanner so `//` or `/*` inside a + * literal stays data. + */ +export function stripComments(text) { + let out = ""; + let i = 0; + while (i < text.length) { + const ch = text[i]; + if (ch === '"' || ch === "'" || ch === "@" || ch === "$") { + const literal = consumeLiteral(text, i); + /* + Mask literal contents too: banned vocabulary inside a string is + data, not code, and must not read as a violation. + */ + for (let j = i; j < literal.end; j++) { + out += text[j] === "\n" ? "\n" : " "; + } + i = literal.end; + continue; + } + + if (ch === "/" && text[i + 1] === "/") { + while (i < text.length && text[i] !== "\n") { + out += " "; + i++; + } + continue; + } + + if (ch === "/" && text[i + 1] === "*") { + while (i < text.length && !(text[i] === "*" && text[i + 1] === "/")) { + out += text[i] === "\n" ? "\n" : " "; + i++; + } + out += " "; + i += 2; + continue; + } + + out += ch; + i++; + } + + return out; +} + const USING_PATTERN = /\busing\s+System\s*\.\s*Linq\b/; const QUALIFIED_PATTERN = /\bSystem\s*\.\s*Linq\s*\./; const ENUMERABLE_PATTERN = /\bEnumerable\s*\.\s*\w+/; -function isCommentOnly(line) { - const trimmed = line.trimStart(); - return ( - trimmed.startsWith("//") || - trimmed.startsWith("/*") || - trimmed.startsWith("*") - ); -} - /** Returns one-based line numbers whose code (not comment) text violates the ban. */ -export function linqViolations(text) { +export function linqViolations(rawText) { + const text = stripComments(rawText); const lines = text.split("\n"); const violations = []; for (let index = 0; index < lines.length; index++) { const line = lines[index]; - if (isCommentOnly(line)) { - continue; - } - if ( USING_PATTERN.test(line) || QUALIFIED_PATTERN.test(line) || diff --git a/tooling~/scripts/tests/lint-linq-production.test.mjs b/tooling~/scripts/tests/lint-linq-production.test.mjs index 40482928..d4cc308a 100644 --- a/tooling~/scripts/tests/lint-linq-production.test.mjs +++ b/tooling~/scripts/tests/lint-linq-production.test.mjs @@ -68,6 +68,26 @@ const EXEMPT = [ "string[] copy = list.ToArray(); // copied without LINQ", ], + [ + "a block comment mentioning the banned vocabulary", + "/*\n using System.Linq is banned; Enumerable.Range allocates.\n*/\nint x = 0;", + ], + + [ + "a one-line block comment with code around it", + "int y = 1; /* Enumerable.Range here is data */ int z = 2;", + ], + + [ + "banned vocabulary inside a string literal", + 'string note = "using System.Linq is banned";', + ], + + [ + "banned vocabulary inside a string after a block comment opens mid-line", + "int a = 0; /* note: Enumerable.Empty */", + ], + [ "a type whose name merely contains Enumerable", "sealed class EnumerableFactory\n{\n void Make() { }\n}", From 03153238fc151bf3624e7cf74359b963c3827ee7 Mon Sep 17 00:00:00 2001 From: wallstop Date: Fri, 11 Sep 2026 21:48:30 +0000 Subject: [PATCH 7/8] Fix theme popup friendly names and linter failed-literal handling Bugbot round on 8f66994: - FriendlyThemeNames passes FriendlyThemeName as its transform; the identity delegate dropped the -theme/theme- strip from the popup. - stripComments null-checks consumeLiteral (verbatim identifiers like @event, dangling quotes copy verbatim instead of throwing); 2 new contract cases pin both shapes. PlayMode 199/199 on Unity 6000.4.6f1; tooling 132 pass. --- Editor/CustomEditors/TerminalUIEditor.cs | 2 +- tooling~/scripts/lint-linq-production.mjs | 11 +++++++++++ tooling~/scripts/tests/lint-linq-production.test.mjs | 10 ++++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/Editor/CustomEditors/TerminalUIEditor.cs b/Editor/CustomEditors/TerminalUIEditor.cs index 4dc08f98..973de3c8 100644 --- a/Editor/CustomEditors/TerminalUIEditor.cs +++ b/Editor/CustomEditors/TerminalUIEditor.cs @@ -611,7 +611,7 @@ private static string[] FriendlyThemeNames(List themeNames) { return RefreshCache( themeNames, - static name => name, + static name => FriendlyThemeName(name), ref _themeDisplayNameSource, ref _themeDisplayNames ); diff --git a/tooling~/scripts/lint-linq-production.mjs b/tooling~/scripts/lint-linq-production.mjs index 948a7030..44c2a18b 100644 --- a/tooling~/scripts/lint-linq-production.mjs +++ b/tooling~/scripts/lint-linq-production.mjs @@ -58,6 +58,17 @@ export function stripComments(text) { const ch = text[i]; if (ch === '"' || ch === "'" || ch === "@" || ch === "$") { const literal = consumeLiteral(text, i); + if (literal === null) { + /* + A quote-like character that does not open a string (verbatim + identifiers such as `@event`, a dangling quote): copy the + character verbatim and keep scanning. + */ + out += ch; + i++; + continue; + } + /* Mask literal contents too: banned vocabulary inside a string is data, not code, and must not read as a violation. diff --git a/tooling~/scripts/tests/lint-linq-production.test.mjs b/tooling~/scripts/tests/lint-linq-production.test.mjs index d4cc308a..9449ca7a 100644 --- a/tooling~/scripts/tests/lint-linq-production.test.mjs +++ b/tooling~/scripts/tests/lint-linq-production.test.mjs @@ -88,6 +88,16 @@ const EXEMPT = [ "int a = 0; /* note: Enumerable.Empty */", ], + [ + "a verbatim identifier that is not a string", + "T value = @event;", + ], + + [ + "an unterminated quote", + 'string never = "unterminated', + ], + [ "a type whose name merely contains Enumerable", "sealed class EnumerableFactory\n{\n void Make() { }\n}", From 42f9b37e3bf683284faf1d2bd9ac0a7c2b1f5ecf Mon Sep 17 00:00:00 2001 From: wallstop Date: Sat, 12 Sep 2026 00:09:46 +0000 Subject: [PATCH 8/8] Add a completeness test for the built-in parser table Review follow-up on 0315323: 'ensure we have tests to verify that all built-in types are registered.' - CommandArg exposes BuiltInParserTypes (internal static property over the table's keys; callers cannot mutate it). - New BuiltInParserTableCoversEveryBuiltInType PlayMode test: the table must match the expected type list exactly (a removed or stale entry fails the count/membership), and each row drives the untyped TryGet(Type, out object) path with a representative input plus a junk-rejection check, so a table entry that stops parsing is caught here even without a dedicated per-type test. Adding a built-in type without a table entry and a row here fails this test. PlayMode 200/200 on Unity 6000.4.6f1. --- Runtime/CommandTerminal/Backend/CommandArg.cs | 6 + Tests/Runtime/CommandArgTests.cs | 233 ++++++++++++++++++ 2 files changed, 239 insertions(+) diff --git a/Runtime/CommandTerminal/Backend/CommandArg.cs b/Runtime/CommandTerminal/Backend/CommandArg.cs index cbbc6a16..af1248cc 100644 --- a/Runtime/CommandTerminal/Backend/CommandArg.cs +++ b/Runtime/CommandTerminal/Backend/CommandArg.cs @@ -21,6 +21,12 @@ public readonly struct CommandArg /// internal delegate bool UntypedParser(string input, out object parsed); + /// + /// Types the built-in parser table covers. Internal for test + /// coverage; callers cannot mutate the returned collection. + /// + internal static IReadOnlyCollection BuiltInParserTypes => BuiltInParsers.Keys; + public static readonly HashSet Delimiters = new() { ',', ';', ':', '_', '/', '\\' }; public static readonly List Quotes = new() { '"', '\'' }; public static readonly HashSet IgnoredValuesForCleanedTypes = new() { "\r", "\n" }; diff --git a/Tests/Runtime/CommandArgTests.cs b/Tests/Runtime/CommandArgTests.cs index 4fed1ced..dd535652 100644 --- a/Tests/Runtime/CommandArgTests.cs +++ b/Tests/Runtime/CommandArgTests.cs @@ -3057,6 +3057,239 @@ public void Ray() Assert.IsFalse(arg.TryGet(out value), $"Unexpectedly parsed {value}"); } + /* + The built-in parser table must contain exactly the expected types: + a removed entry fails the membership check, a stale entry fails the + count, and an entry whose delegate was lost fails the parse. Each + row drives the untyped TryGet(Type, out object) path with one + representative input and pins the parsed value, so a table entry + that stops working is caught here even without a dedicated test. + Adding a built-in type means adding its table entry AND a row here. + */ + [Test] + public void BuiltInParserTableCoversEveryBuiltInType() + { + (Type type, string input, Func matches)[] rows = + { + (typeof(bool), "true", parsed => Equals(parsed, true)), + (typeof(float), "1.5", parsed => Approximately(1.5f, (float)parsed)), + (typeof(int), "7", parsed => Equals(parsed, 7)), + (typeof(uint), "9", parsed => Equals(parsed, 9u)), + (typeof(long), "11", parsed => Equals(parsed, 11L)), + (typeof(ulong), "13", parsed => Equals(parsed, 13ul)), + (typeof(double), "2.5", parsed => Approximately(2.5d, (double)parsed)), + (typeof(short), "3", parsed => Equals(parsed, (short)3)), + (typeof(ushort), "5", parsed => Equals(parsed, (ushort)5)), + (typeof(byte), "2", parsed => Equals(parsed, (byte)2)), + (typeof(sbyte), "-2", parsed => Equals(parsed, (sbyte)-2)), + ( + typeof(Guid), + "6f9619ff-8b86-d011-b42d-00c04fc964ff", + parsed => Equals(parsed, new Guid("6f9619ff-8b86-d011-b42d-00c04fc964ff")) + ), + ( + typeof(DateTime), + "2026-01-02T03:04:05.0000000Z", + parsed => + Equals( + parsed, + new DateTime(2026, 1, 2, 3, 4, 5, DateTimeKind.Utc).ToLocalTime() + ) || Equals(parsed, new DateTime(2026, 1, 2, 3, 4, 5, DateTimeKind.Utc)) + ), + ( + typeof(DateTimeOffset), + "2026-01-02T03:04:05+00:00", + parsed => Equals(parsed, new DateTimeOffset(2026, 1, 2, 3, 4, 5, TimeSpan.Zero)) + ), + (typeof(char), "x", parsed => Equals(parsed, 'x')), + (typeof(decimal), "1.5", parsed => Equals(parsed, 1.5m)), + ( + typeof(System.Numerics.BigInteger), + "42", + parsed => Equals(parsed, new System.Numerics.BigInteger(42)) + ), + (typeof(TimeSpan), "00:00:03", parsed => Equals(parsed, TimeSpan.FromSeconds(3))), + (typeof(Version), "1.2", parsed => Equals(parsed, new Version(1, 2))), + ( + typeof(System.Net.IPAddress), + "127.0.0.1", + parsed => Equals(parsed, System.Net.IPAddress.Loopback) + ), + ( + typeof(Vector2), + "1.5, 2.5", + parsed => + { + Vector2 vector = (Vector2)parsed; + return Approximately(1.5f, vector.x) && Approximately(2.5f, vector.y); + } + ), + ( + typeof(Vector3), + "1.5, 2.5, 3.5", + parsed => + { + Vector3 vector = (Vector3)parsed; + return Approximately(1.5f, vector.x) + && Approximately(2.5f, vector.y) + && Approximately(3.5f, vector.z); + } + ), + ( + typeof(Vector4), + "1.5, 2.5, 3.5, 4.5", + parsed => + { + Vector4 vector = (Vector4)parsed; + return Approximately(1.5f, vector.x) + && Approximately(2.5f, vector.y) + && Approximately(3.5f, vector.z) + && Approximately(4.5f, vector.w); + } + ), + (typeof(Vector2Int), "1, 2", parsed => Equals(parsed, new Vector2Int(1, 2))), + (typeof(Vector3Int), "1, 2, 3", parsed => Equals(parsed, new Vector3Int(1, 2, 3))), + ( + typeof(Color), + "1, 0, 0, 1", + parsed => + { + Color color = (Color)parsed; + return Approximately(1f, color.r) + && Approximately(0f, color.g) + && Approximately(0f, color.b) + && Approximately(1f, color.a); + } + ), + ( + typeof(Quaternion), + "0, 0, 0, 1", + parsed => + { + Quaternion quaternion = (Quaternion)parsed; + return Approximately(0f, quaternion.x) + && Approximately(0f, quaternion.y) + && Approximately(0f, quaternion.z) + && Approximately(1f, quaternion.w); + } + ), + (typeof(Rect), "1, 2, 3, 4", parsed => Equals(parsed, new Rect(1f, 2f, 3f, 4f))), + (typeof(RectInt), "1, 2, 3, 4", parsed => Equals(parsed, new RectInt(1, 2, 3, 4))), + ( + typeof(Bounds), + "1, 2, 3, 4, 5, 6", + parsed => + { + Bounds bounds = (Bounds)parsed; + return Approximately(1f, bounds.center.x) + && Approximately(2f, bounds.center.y) + && Approximately(3f, bounds.center.z) + && Approximately(4f, bounds.size.x) + && Approximately(5f, bounds.size.y) + && Approximately(6f, bounds.size.z); + } + ), + ( + typeof(BoundsInt), + "1, 2, 3, 4, 5, 6", + parsed => + { + BoundsInt bounds = (BoundsInt)parsed; + return bounds.position.x == 1 + && bounds.position.y == 2 + && bounds.position.z == 3 + && bounds.size.x == 4 + && bounds.size.y == 5 + && bounds.size.z == 6; + } + ), + ( + typeof(RectOffset), + "4, 8, 2, 6", + parsed => + { + RectOffset offset = (RectOffset)parsed; + return offset.left == 4 + && offset.right == 8 + && offset.top == 2 + && offset.bottom == 6; + } + ), + ( + typeof(Plane), + "0, 1, 0, 5", + parsed => + { + Plane plane = (Plane)parsed; + return Approximately(0f, plane.normal.x) + && Approximately(1f, plane.normal.y) + && Approximately(0f, plane.normal.z) + && Approximately(5f, plane.distance); + } + ), + ( + typeof(Ray), + "1, 2, 3, 0, 0, -1", + parsed => + { + Ray ray = (Ray)parsed; + return Approximately(1f, ray.origin.x) + && Approximately(2f, ray.origin.y) + && Approximately(3f, ray.origin.z) + && Approximately(0f, ray.direction.x) + && Approximately(0f, ray.direction.y) + && Approximately(-1f, ray.direction.z); + } + ), + ( + typeof(System.Numerics.Complex), + "1.5, -2.5", + parsed => Equals(parsed, new System.Numerics.Complex(1.5, -2.5)) + ), + }; + + /* + Table membership first: the expected type list must match the + table exactly, so a removed entry or an unregistered new + built-in fails here before any parse runs. + */ + Type[] expectedTypes = rows.Select(row => row.type).ToArray(); + Type[] registeredTypes = CommandArg.BuiltInParserTypes.ToArray(); + Assert.AreEqual( + expectedTypes.Length, + registeredTypes.Length, + "Built-in parser table size drifted; expected: [" + + $"{string.Join(", ", expectedTypes.Select(type => type.Name))}], " + + $"registered: [{string.Join(", ", registeredTypes.Select(type => type.Name))}]" + ); + foreach (Type expectedType in expectedTypes) + { + Assert.IsTrue( + registeredTypes.Contains(expectedType), + $"{expectedType.Name} is missing from the built-in parser table" + ); + } + + foreach ((Type type, string input, Func matches) in rows) + { + CommandArg arg = new(input); + Assert.IsTrue( + arg.TryGet(type, out object parsed), + $"{type.Name} is in the built-in table but failed to parse '{input}'" + ); + Assert.IsTrue( + matches(parsed), + $"{type.Name} parsed '{input}' to {parsed}, which does not match the expected value" + ); + + arg = new CommandArg("asdf-junk"); + Assert.IsFalse( + arg.TryGet(type, out object junk), + $"{type.Name} unexpectedly parsed junk as {junk}" + ); + } + } + [Test] public void Untyped() {