[UUM-138261] Fix move handle exponentially moving the selected elements

[lopezt-unity]

Purpose of this PR

This PR fixes an issue with the Move handle, creating a accumulation of the move deltas when using the handles. This was resulting in an exponential motion of the selected element.

This was caused by a wrong test in the rotation handle type, causing a cache invalidation in the middle of the process. This PR fixes that and focuses on getting PivotRotation.Local and World from unity as they are the only valid ones. This prevent that the use of new pivot types (such as grid or custom) messes up with ProBuilder state.

This PR also added tests to validate that PB pivot types are aligned with these changes.

Links

Jira: https://jira.unity3d.com/browse/UUM-138261

Comments to Reviewers

[List known issues, planned work, provide any extra context for your code.]

[u-pr]

This is a solid and clean implementation. A minor enhancement is suggested to cache a static Unity property to avoid redundant native-to-managed bridge transitions.

_{🤖 Helpful? 👍/👎}

[u-pr]

Have you considered caching Tools.pivotRotation into a local variable at the beginning of SyncPivotRotation()?

Since Tools is a Unity class, accessing Tools.pivotRotation repeatedly (up to 7 times in this method) likely involves native-to-managed bridge transitions. Caching it once makes the code cleaner, guarantees consistency throughout the method execution, and avoids redundant native property lookups.

For example:

static void SyncPivotRotation()
{
    var activePivotRotation = Tools.pivotRotation;
    if (s_PivotRotation != activePivotRotation)
    {
        s_HandleOrientation.SetValue(activePivotRotation == PivotRotation.Local
            ? HandleOrientation.ActiveObject
            : HandleOrientation.World);
        s_PivotRotation = activePivotRotation;
        MeshSelection.InvalidateCaches();
        pivotRotationChanged?.Invoke();
        return;
    }
    // ... use activePivotRotation below
}

_{🤖 Helpful? 👍/👎 by guardian}

[codecov-github-com]

Codecov Report

All modified and coverable lines are covered by tests ✅

@@            Coverage Diff             @@
##           master     #676      +/-   ##
==========================================
+ Coverage   37.96%   38.05%   +0.08%     
==========================================
  Files         278      278              
  Lines       38851    38915      +64     
==========================================
+ Hits        14751    14808      +57     
- Misses      24100    24107       +7     

Flag	Coverage Δ
probuilder_MacOS_6000.0	35.69% <100.00%> (+0.04%)	⬆️
probuilder_MacOS_6000.3	35.69% <100.00%> (+0.04%)	⬆️
probuilder_MacOS_6000.4	35.68% <100.00%> (+0.04%)	⬆️
probuilder_MacOS_6000.5	35.69% <100.00%> (+0.04%)	⬆️
probuilder_MacOS_6000.6	35.69% <100.00%> (+0.04%)	⬆️
probuilder_Windows_6000.0	35.61% <100.00%> (+0.37%)	⬆️
probuilder_Windows_6000.3	35.61% <100.00%> (+0.36%)	⬆️
probuilder_Windows_6000.4	35.60% <100.00%> (+0.36%)	⬆️
probuilder_Windows_6000.5	35.61% <100.00%> (+0.36%)	⬆️
probuilder_Windows_6000.6	35.61% <100.00%> (?)

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

Files with missing lines	Coverage Δ
Editor/EditorCore/VertexManipulationTool.cs	39.58% <100.00%> (+6.06%)	⬆️

... and 2 files with indirect coverage changes

ℹ️ Need help interpreting these results?