Skip to content

FIX: Handle consecutive wildcards in StringMatches (2/x) #2311

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
josepmariapujol-unity wants to merge 7 commits into
base: develop
Choose a base branch
from
Open

FIX: Handle consecutive wildcards in StringMatches (2/x) #2311

josepmariapujol-unity wants to merge 7 commits into from
+33 −4

Conversation

@josepmariapujol-unity
Copy link
Collaborator

@josepmariapujol-unity josepmariapujol-unity commented Jan 7, 2026 •
edited
Loading

Description

Normalize consecutive wildcards so /* semantics are handled correctly

JIRA: WIP

Testing status & QA

Please describe the testing already done by you and what testing you request/recommend QA to execute. If you used or created any testing project please link them here too for QA.

Overall Product Risks

Please rate the potential complexity and halo effect from low to high for the reviewers. Note down potential risks to specific Editor branches if any.

  • Complexity:
  • Halo Effect:

Comments to reviewers

Please describe any additional information such as what to focus on, or historical info for the reviewers.

Checklist

Before review:

  • Changelog entry added.
    • Explains the change in Changed, Fixed, Added sections.
    • For API change contains an example snippet and/or migration example.
    • JIRA ticket linked, example (case %%). If it is a private issue, just add the case ID without a link.
    • Jira port for the next release set as "Resolved".
  • Tests added/changed, if applicable.
    • Functional tests Area_CanDoX, Area_CanDoX_EvenIfYIsTheCase, Area_WhenIDoX_AndYHappens_ThisIsTheResult.
    • Performance tests.
    • Integration tests.
  • Docs for new/changed API's.
    • Xmldoc cross references are set correctly.
    • Added explanation how the API works.
    • Usage code examples added.
    • The manual is updated, if needed.

During merge:

  • Commit message for squash-merge is prefixed with one of the list:
    • NEW: ___.
    • FIX: ___.
    • DOCS: ___.
    • CHANGE: ___.
    • RELEASE: 1.1.0-preview.3.

@josepmariapujol-unity josepmariapujol-unity self-assigned this Jan 7, 2026
@u-pr-agent
Copy link
Contributor

u-pr-agent bot commented Jan 7, 2026 •
edited
Loading

PR Reviewer Guide 🔍

(Review updated until commit 322b52e)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

Small, localized behavior change in path matching plus a focused test and changelog update.
🏅 Score: 86

The fix is straightforward and test-backed, but visibility changes and wildcard semantics should be validated against existing callers and expected `*` vs `**` behavior.
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

API Surface

PathComponentType and MatchPathComponent were changed from private to internal; confirm this is intentional (tests vs. product code) and won’t create unwanted external dependencies or API-compat expectations within the assembly.

 
internal enum PathComponentType
{
    Name,
    DisplayName,
    Usage,
    Layout
}

internal static bool MatchPathComponent(string component, string path, ref int indexInPath, PathComponentType componentType, int startIndexInComponent = 0)
{
Semantics Change

Collapsing consecutive * changes behavior by making **/*** equivalent to *; verify this matches desired semantics for all component types (Name/Layout/Usage/DisplayName) and doesn’t conflict with any prior or planned special meaning for **.

 
// If we've reached a '*' in the path, skip character in name.
if (nextCharInPath == '*')
{
    // Collapse consecutive '*' so matching logic here only needs to handle a single '*'.
    while (indexInPath + 1 < pathLength && path[indexInPath + 1] == '*')
        ++indexInPath;

    // But first let's see if we have something after the wildcard that matches the rest of the component.
Test Coverage

The new test only asserts match success for *Trigger, **Trigger, ***Trigger; consider adding assertions for non-matches and edge cases (e.g., patterns consisting only of */**, wildcard at end, and interaction with delimiters) to prevent regressions in indexInPath advancement and termination conditions.

 
[Test]
[Category("Controls")]
public void Controls_MatchPathComponent_CollapsesConsecutiveWildcards()
{
    var component = "leftTrigger";
    var componentType = InputControlPath.PathComponentType.Name;

    var patterns = new[]
    {
        "*Trigger",
        "**Trigger",
        "***Trigger"
    };

    foreach (var path in patterns)
    {
        var indexInPath = 0;
        var result = InputControlPath.MatchPathComponent(component, path, ref indexInPath, componentType);

        // All patterns should match
        Assert.IsTrue(result, $"Pattern '{path}' should match '{component}'");
        Assert.AreEqual(path.Length, indexInPath,
            $"Index should advance past entire pattern for '{path}'");
    }
  • Update review

🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr-agent

@josepmariapujol-unity josepmariapujol-unity changed the title Normalize consecutive wildcards so */** semantics are handled correctly Handle consecutive wildcards in StringMatches (2/x) Jan 7, 2026
@u-pr-agent
Copy link
Contributor

u-pr-agent bot commented Jan 7, 2026

PR Code Suggestions ✨

No code suggestions found for the PR.

@codecov-github-com
Copy link

codecov-github-com bot commented Jan 7, 2026 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

@@           Coverage Diff            @@
##           develop    #2311   +/-   ##
========================================
  Coverage    77.95%   77.95%           
========================================
  Files          476      476           
  Lines        97443    97454   +11     
========================================
+ Hits         75961    75972   +11     
  Misses       21482    21482
Flag Coverage Δ
inputsystem_MacOS_2022.3 5.54% <0.00%> (-0.01%) ⬇️
inputsystem_MacOS_2022.3_project 75.47% <100.00%> (+<0.01%) ⬆️
inputsystem_MacOS_6000.0 5.32% <0.00%> (-0.01%) ⬇️
inputsystem_MacOS_6000.0_project 77.36% <100.00%> (+<0.01%) ⬆️
inputsystem_MacOS_6000.2 5.32% <0.00%> (-0.01%) ⬇️
inputsystem_MacOS_6000.2_project 77.36% <100.00%> (+<0.01%) ⬆️
inputsystem_MacOS_6000.3 5.32% <0.00%> (-0.01%) ⬇️
inputsystem_MacOS_6000.3_project 77.36% <100.00%> (-0.01%) ⬇️
inputsystem_MacOS_6000.4 5.32% <0.00%> (-0.01%) ⬇️
inputsystem_MacOS_6000.4_project 77.37% <100.00%> (+0.01%) ⬆️
inputsystem_MacOS_6000.5 5.32% <0.00%> (-0.01%) ⬇️
inputsystem_MacOS_6000.5_project 77.37% <100.00%> (+<0.01%) ⬆️
inputsystem_Ubuntu_2022.3 5.54% <0.00%> (-0.01%) ⬇️
inputsystem_Ubuntu_2022.3_project 75.27% <100.00%> (+<0.01%) ⬆️
inputsystem_Ubuntu_6000.0 5.32% <0.00%> (-0.01%) ⬇️
inputsystem_Ubuntu_6000.0_project 77.16% <100.00%> (+<0.01%) ⬆️
inputsystem_Ubuntu_6000.2 5.32% <0.00%> (-0.01%) ⬇️
inputsystem_Ubuntu_6000.2_project 77.17% <100.00%> (+0.01%) ⬆️
inputsystem_Ubuntu_6000.3 5.32% <0.00%> (-0.01%) ⬇️
inputsystem_Ubuntu_6000.3_project 77.16% <100.00%> (+<0.01%) ⬆️
inputsystem_Ubuntu_6000.4 5.33% <0.00%> (-0.01%) ⬇️
inputsystem_Ubuntu_6000.4_project 77.17% <100.00%> (+<0.01%) ⬆️
inputsystem_Ubuntu_6000.5 5.33% <0.00%> (-0.01%) ⬇️
inputsystem_Ubuntu_6000.5_project 77.18% <100.00%> (+<0.01%) ⬆️
inputsystem_Windows_2022.3 5.54% <0.00%> (-0.01%) ⬇️
inputsystem_Windows_2022.3_project 75.60% <100.00%> (+<0.01%) ⬆️
inputsystem_Windows_6000.0 5.32% <0.00%> (-0.01%) ⬇️
inputsystem_Windows_6000.0_project 77.49% <100.00%> (+<0.01%) ⬆️
inputsystem_Windows_6000.2 5.32% <0.00%> (-0.01%) ⬇️
inputsystem_Windows_6000.2_project 77.49% <100.00%> (+<0.01%) ⬆️
inputsystem_Windows_6000.3 5.32% <0.00%> (-0.01%) ⬇️
inputsystem_Windows_6000.3_project 77.49% <100.00%> (+<0.01%) ⬆️
inputsystem_Windows_6000.4 5.32% <0.00%> (-0.01%) ⬇️
inputsystem_Windows_6000.4_project 77.50% <100.00%> (+<0.01%) ⬆️
inputsystem_Windows_6000.5 5.32% <0.00%> (-0.01%) ⬇️
inputsystem_Windows_6000.5_project 77.50% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
Assets/Tests/InputSystem/CoreTests_Controls.cs 99.81% <100.00%> (+<0.01%) ⬆️
...putsystem/InputSystem/Controls/InputControlPath.cs 91.87% <100.00%> (-0.01%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@josepmariapujol-unity josepmariapujol-unity changed the title Handle consecutive wildcards in StringMatches (2/x) FIX: Handle consecutive wildcards in StringMatches (2/x) Jan 8, 2026
ekcoh
ekcoh requested changes Jan 12, 2026
View reviewed changes
Copy link
Collaborator

@ekcoh ekcoh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change looks good to me but I would suggest making sure this is covered in a unit test. If this logic currently lacks any unit test I would suggest adding at least one covering this specific scenario.

ekcoh
ekcoh reviewed Jan 12, 2026
View reviewed changes
Copy link
Collaborator

@ekcoh ekcoh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another thing to address, there is no ticket trace nor any CHANGELOG entry for this fix, please add that.

@josepmariapujol-unity josepmariapujol-unity marked this pull request as draft January 12, 2026 14:01
@josepmariapujol-unity josepmariapujol-unity marked this pull request as ready for review January 12, 2026 14:29
@u-pr-agent
Copy link
Contributor

u-pr-agent bot commented Jan 12, 2026

Persistent review updated to latest commit 322b52e

@u-pr-agent
Copy link
Contributor

u-pr-agent bot commented Jan 12, 2026

PR Code Suggestions ✨

No code suggestions found for the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ekcoh ekcoh Awaiting requested review from ekcoh

Requested changes must be addressed to merge this pull request.

Assignees

@josepmariapujol-unity josepmariapujol-unity

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@josepmariapujol-unity @ekcoh