Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates DxCommandTerminal to support per-command opt-out from history recording, fixes default-command registration behavior when ignoreDefaultCommands is enabled, and removes the bundled System.Collections.Immutable DLL by replacing immutable sets with an internal read-only hash set wrapper.
Changes:
- Add
RegisterCommandAttribute.AddToHistory/CommandInfo.addToHistory/CommandShell.AddCommand(..., addToHistory)and stop recordingclear-historyin history. - Replace
ImmutableHashSet<string>usages withReadOnlyHashSet<string>and remove the bundledSystem.Collections.Immutableplugin assets. - Add/expand runtime tests covering argument parsing, history behavior, keyboard controller control-order integrity checks, and
ignoreDefaultCommands.
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Tests/Runtime/TryEatArgumentTests.cs (+.meta) | New tests validating CommandShell.TryEatArgument parsing and remainder behavior. |
| Tests/Runtime/TerminalTests.cs | Strengthens assertions and adds coverage for ignoreDefaultCommands behavior. |
| Tests/Runtime/TerminalKeyboardControllerTests.cs (+.meta) | New tests validating control-order integrity warnings via reflection. |
| Tests/Runtime/CommandShellTests.cs | Improves assertion messages (no functional behavior change). |
| Tests/Runtime/CommandHistoryTests.cs (+.meta) | New unit/integration tests for CommandHistory.Count, Clear(), and clear-history behavior. |
| Runtime/DataStructures/ReadOnlyHashSet.cs (+.meta) | Introduces ReadOnlyHashSet<T> wrapper used to replace immutables dependency. |
| Runtime/DataStructures/ReadOnlyHashSetExtensions.cs (+.meta) | Adds ToReadOnlyHashSet helper for snapshot creation. |
| Runtime/CommandTerminal/Input/TerminalKeyboardController.cs | Adjusts integrity check to warn only on missing control types (not duplicates). |
| Runtime/CommandTerminal/Backend/CommandShell.cs | Wires AddToHistory into command registration and (partially) into history recording. |
| Runtime/CommandTerminal/Backend/CommandInfo.cs | Adds addToHistory field with default true. |
| Runtime/CommandTerminal/Backend/CommandHistory.cs | Adds Count property delegating to the underlying buffer. |
| Runtime/CommandTerminal/Backend/BuiltinCommands.cs | Marks clear-history as AddToHistory = false. |
| Runtime/Attributes/RegisterCommandAttribute.cs | Adds AddToHistory and fixes Default propagation in the internal ctor. |
| Runtime/Plugins.meta / Runtime/Plugins/System.Collections.Immutable.dll.meta | Removes Unity plugin metadata for the bundled Immutable DLL. |
| package.json | Bumps package version to 1.0.0-rc25.0. |
| CHANGELOG.md | Converts to Keep a Changelog format and documents the new behavior/removals. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 23 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 25 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 25 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 26 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
Runtime/CommandTerminal/Input/TerminalKeyboardController.cs:155
- VerifyControlOrderIntegrity now only warns when controls are missing, but it no longer warns when
_controlOrdercontains entries that are not inControlTypes(e.g.,TerminalControlTypes.Noneor any out-of-range serialized enum value). Those extra entries will be silently ignored inUpdate()(no_inputChecksmapping), which makes inspector misconfiguration harder to detect. Consider also detectingextraControls = _controlOrder.Except(ControlTypes)and warning (or reverting to a SetEquals-style check but with clearer messaging for both missing and extra controls).
private void VerifyControlOrderIntegrity()
{
TerminalControlTypes[] missingControls = ControlTypes.Except(_controlOrder).ToArray();
if (missingControls.Length > 0)
{
Debug.LogWarning(
$"Control Order is missing the following controls: [{string.Join(", ", missingControls)}]. "
+ "Input for these will not be handled. Is this intentional?",
this
);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
✓ Fix
Systems.Immutablecascading dependency issue✓ Fix
clear-historyadding itself to history✓ Fix extraneous warnings