From 1e25f10d7fc3684724566cc20f706d3a6a64ef16 Mon Sep 17 00:00:00 2001 From: Thomas Lopez Date: Wed, 27 May 2026 15:14:34 -0400 Subject: [PATCH 1/5] fixed polyshape tool not being able to snap to the grid --- Editor/EditorCore/PolyShapeTool.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Editor/EditorCore/PolyShapeTool.cs b/Editor/EditorCore/PolyShapeTool.cs index 81ab5e431..138284e34 100644 --- a/Editor/EditorCore/PolyShapeTool.cs +++ b/Editor/EditorCore/PolyShapeTool.cs @@ -236,6 +236,8 @@ static bool CanCreateNewPolyShape() [EditorTool("Edit PolyShape", typeof(PolyShape))] public class PolyShapeTool : EditorTool { + public override bool gridSnapEnabled => true; + [MenuItem("Tools/ProBuilder/Edit/Edit PolyShape", true, PreferenceKeys.menuEditor + 10)] static bool ValidateEditShapeTool() { From 6f00061134808945ebda0cb70eb42346dd038c74 Mon Sep 17 00:00:00 2001 From: Thomas Lopez Date: Wed, 27 May 2026 15:15:07 -0400 Subject: [PATCH 2/5] fix pbshape creation not snapping to the grid --- Editor/StateMachines/ShapeState_InitShape.cs | 85 +++++++++----------- 1 file changed, 40 insertions(+), 45 deletions(-) diff --git a/Editor/StateMachines/ShapeState_InitShape.cs b/Editor/StateMachines/ShapeState_InitShape.cs index 9e8e9d347..68298898f 100644 --- a/Editor/StateMachines/ShapeState_InitShape.cs +++ b/Editor/StateMachines/ShapeState_InitShape.cs @@ -47,59 +47,54 @@ public override ShapeState DoState(Event evt) if(evt.isMouse && HandleUtility.nearestControl == tool.controlID) { - if (evt.type != EventType.MouseDown) - { - HandleUtility.PlaceObject(evt.mousePosition, out m_HitPosition, out _); - m_HitPosition = tool.GetPoint(m_HitPosition); - } - else + var res = EditorHandleUtility.FindBestPlaneAndBitangent(evt.mousePosition, tool.m_DuplicateGO); + Ray ray = HandleUtility.GUIPointToWorldRay(evt.mousePosition); + float hit; + + if (res.item1.Raycast(ray, out hit)) { - var res = EditorHandleUtility.FindBestPlaneAndBitangent(evt.mousePosition, tool.m_DuplicateGO); - Ray ray = HandleUtility.GUIPointToWorldRay(evt.mousePosition); - float hit; + var planeNormal = res.item1.normal; + var planeCenter = res.item1.normal * -res.item1.distance; + + // if hit point on plane is cardinal axis and on grid, snap to grid. + if (Math.IsCardinalAxis(planeNormal)) + { + const float epsilon = .00001f; + bool offGrid = false; + Vector3 snapVal = EditorSnapping.activeMoveSnapValue; + Vector3 center = + Vector3.Scale(ProBuilderSnapping.GetSnappingMaskBasedOnNormalVector(planeNormal), + planeCenter); + for (int i = 0; i < 3; i++) + offGrid |= Mathf.Abs(snapVal[i] % center[i]) > epsilon; + tool.m_IsOnGrid = !offGrid; + } + else + { + tool.m_IsOnGrid = false; + } - if (evt.button == 0 && res.item1.Raycast(ray, out hit)) + m_HitPosition = tool.GetPoint(ray.GetPoint(hit)); + + //Click has been done => Define a plane for the tool + if (evt.type == EventType.MouseDown && evt.button == 0) { - //Plane init + //Plane init (persist for subsequent states) tool.m_Plane = res.item1; tool.m_PlaneForward = res.item2; tool.m_PlaneRight = Vector3.Cross(tool.m_Plane.normal, tool.m_PlaneForward); - var planeNormal = tool.m_Plane.normal; - var planeCenter = tool.m_Plane.normal * -tool.m_Plane.distance; - // if hit point on plane is cardinal axis and on grid, snap to grid. - if (Math.IsCardinalAxis(planeNormal)) - { - const float epsilon = .00001f; - bool offGrid = false; - Vector3 snapVal = EditorSnapping.activeMoveSnapValue; - Vector3 center = - Vector3.Scale(ProBuilderSnapping.GetSnappingMaskBasedOnNormalVector(planeNormal), - planeCenter); - for (int i = 0; i < 3; i++) - offGrid |= Mathf.Abs(snapVal[i] % center[i]) > epsilon; - tool.m_IsOnGrid = !offGrid; - } - else - { - tool.m_IsOnGrid = false; - } - - m_HitPosition = tool.GetPoint(ray.GetPoint(hit)); - - //Click has been done => Define a plane for the tool - if (evt.type == EventType.MouseDown && evt.button == 0) - { - //BB init - tool.m_BB_Origin = m_HitPosition; - tool.m_BB_HeightCorner = tool.m_BB_Origin; - tool.m_BB_OppositeCorner = tool.m_BB_Origin; - - return NextState(); - } + //BB init + tool.m_BB_Origin = m_HitPosition; + tool.m_BB_HeightCorner = tool.m_BB_Origin; + tool.m_BB_OppositeCorner = tool.m_BB_Origin; + + return NextState(); } - else - m_HitPosition = Vector3.positiveInfinity; + } + else + { + m_HitPosition = Vector3.positiveInfinity; } } From 65db2ddbc49ee4f9d4410626e1b7b28660a8752b Mon Sep 17 00:00:00 2001 From: Thomas Lopez Date: Wed, 27 May 2026 15:16:16 -0400 Subject: [PATCH 3/5] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a15e0253..dcea8737c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Fixed +- [UUM-141074] Fixed an issue where the CreateShape and CreatePolyshape tools would not properly snap to the grid. - [UUM-133861] Fixed "Look rotation viewing vector is zero" log being spammed when holding shift while using a create tool such as Create Sprite. - [UUM-133859] Fixed an issue in URP projects where the Editor would recompile scripts when after a rectangle selection in ProBuilder. - [UUM-133530] Fixed the `Set Double Sided` custom action in the Editor Sample, which was previously remaining disabled. From 66143e48ade60148408d4bade1a18b53b8d40a60 Mon Sep 17 00:00:00 2001 From: Thomas Lopez Date: Wed, 27 May 2026 15:45:48 -0400 Subject: [PATCH 4/5] adding tests --- Tests/Editor/Editor/ShapeToolGridSnapTests.cs | 151 ++++++++++++++++++ .../Editor/ShapeToolGridSnapTests.cs.meta | 2 + 2 files changed, 153 insertions(+) create mode 100644 Tests/Editor/Editor/ShapeToolGridSnapTests.cs create mode 100644 Tests/Editor/Editor/ShapeToolGridSnapTests.cs.meta diff --git a/Tests/Editor/Editor/ShapeToolGridSnapTests.cs b/Tests/Editor/Editor/ShapeToolGridSnapTests.cs new file mode 100644 index 000000000..3119cbd95 --- /dev/null +++ b/Tests/Editor/Editor/ShapeToolGridSnapTests.cs @@ -0,0 +1,151 @@ +using NUnit.Framework; +using UnityEditor; +using UnityEditor.EditorTools; +using UnityEditor.ProBuilder; +using UnityEngine; +using UnityEngine.ProBuilder; +using ToolManager = UnityEditor.EditorTools.ToolManager; + +public class ShapeToolGridSnapTests +{ + [SetUp] + public void SetUp() + { + ToolManager.SetActiveContext(); + } + + [TearDown] + public void TearDown() + { + ToolManager.RestorePreviousPersistentTool(); + } + + [Test] + public void CreateCubeTool_HasGridSnapEnabled() + { + ToolManager.SetActiveTool(); + var toolType = ToolManager.activeToolType; + + Assert.That(toolType, Is.EqualTo(typeof(CreateCubeTool))); + + // Verify the tool has gridSnapEnabled property set to true + var tool = DrawShapeTool.instance; + Assert.That(tool, Is.Not.Null); + Assert.That(tool.gridSnapEnabled, Is.True); + } + + [Test] + public void DrawShapeTool_GetPoint_SnapsWhenOnGridAndGridSnapActive() + { + ToolManager.SetActiveTool(); + var tool = DrawShapeTool.instance; + Assume.That(tool, Is.Not.Null); + + // Enable grid mode (this is set by ShapeState_InitShape during hover) + tool.m_IsOnGrid = true; + + // Only test if grid snapping is currently active in the editor + if (EditorSnapSettings.gridSnapActive) + { + // Test position that should snap to grid + Vector3 unsnapedPosition = new Vector3(1.23f, 2.34f, 3.45f); + Vector3 snappedPosition = tool.GetPoint(unsnapedPosition); + + // Position should be snapped (not equal to original) + Assert.That(snappedPosition, Is.Not.EqualTo(unsnapedPosition)); + } + else + { + // Skip test if grid snap is not active + Assert.Ignore("Grid snapping is not active in the editor, skipping snap verification"); + } + } + + [Test] + public void DrawShapeTool_GetPoint_DoesNotSnapWhenOffGrid() + { + ToolManager.SetActiveTool(); + var tool = DrawShapeTool.instance; + Assume.That(tool, Is.Not.Null); + + // Disable grid mode + tool.m_IsOnGrid = false; + + // Test position should NOT snap regardless of editor snap settings + Vector3 position = new Vector3(1.23f, 2.34f, 3.45f); + Vector3 result = tool.GetPoint(position); + + // Position should remain unchanged when m_IsOnGrid is false + Assert.That(result, Is.EqualTo(position)); + } + + [Test] + public void PolyShapeTool_HasGridSnapEnabled() + { + // Test that PolyShapeTool also has gridSnapEnabled (fix from commit 1e25f10d7) + ToolManager.SetActiveTool(); + + // PolyShapeTool doesn't have a static instance like DrawShapeTool, + // so we just verify it was activated successfully + Assert.That(ToolManager.activeToolType, Is.EqualTo(typeof(DrawPolyShapeTool))); + } + + [Test] + public void DrawShapeTool_GetPoint_RespectsIncrementalSnap() + { + ToolManager.SetActiveTool(); + var tool = DrawShapeTool.instance; + Assume.That(tool, Is.Not.Null); + + // Get current incremental snap value + Vector3 snapValue = EditorSnapping.incrementalSnapMoveValue; + + // Test with incremental snap enabled + Vector3 unsnappedPosition = new Vector3(1.23f, 2.34f, 3.45f); + Vector3 snappedPosition = tool.GetPoint(unsnappedPosition, useIncrementSnap: true); + + // Position should be snapped to incremental snap value + Assert.That(snappedPosition, Is.Not.EqualTo(unsnappedPosition)); + + // Verify each component is a multiple of snap value (within tolerance) + Assert.That(snappedPosition.x % snapValue.x, Is.EqualTo(0).Within(0.001f)); + Assert.That(snappedPosition.y % snapValue.y, Is.EqualTo(0).Within(0.001f)); + Assert.That(snappedPosition.z % snapValue.z, Is.EqualTo(0).Within(0.001f)); + } + + /// Test for UUM-141074: + [Test] + public void ShapeState_InitShape_CalculatesGridStateBeforeMouseDown() + { + // This test verifies the fix for UUM-141074 + // Before the fix, m_IsOnGrid was only calculated on MouseDown + // After the fix, it's calculated during hover (before MouseDown) + + ToolManager.SetActiveTool(); + var tool = DrawShapeTool.instance; + Assume.That(tool, Is.Not.Null); + + // The fix ensures that GetPoint can snap correctly during hover + // because m_IsOnGrid is set before GetPoint is called + // We can't directly test the state machine, but we can verify + // that the tool's GetPoint method respects the m_IsOnGrid flag + + // Test 1: When m_IsOnGrid is true, GetPoint should snap (if grid is active) + tool.m_IsOnGrid = true; + Vector3 pos1 = new Vector3(1.23f, 2.34f, 3.45f); + Vector3 result1 = tool.GetPoint(pos1); + + if (EditorSnapSettings.gridSnapActive) + { + Assert.That(result1, Is.Not.EqualTo(pos1), + "GetPoint should snap when m_IsOnGrid is true and grid snap is active"); + } + + // Test 2: When m_IsOnGrid is false, GetPoint should NOT snap + tool.m_IsOnGrid = false; + Vector3 pos2 = new Vector3(1.23f, 2.34f, 3.45f); + Vector3 result2 = tool.GetPoint(pos2); + Assert.That(result2, Is.EqualTo(pos2), + "GetPoint should not snap when m_IsOnGrid is false"); + } +} diff --git a/Tests/Editor/Editor/ShapeToolGridSnapTests.cs.meta b/Tests/Editor/Editor/ShapeToolGridSnapTests.cs.meta new file mode 100644 index 000000000..f112cffd4 --- /dev/null +++ b/Tests/Editor/Editor/ShapeToolGridSnapTests.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 41c2a001abcc39e439cbf0c7a5d4df2b \ No newline at end of file From b98b7f9ff5222e834da9bf51160ce207fd78b5c1 Mon Sep 17 00:00:00 2001 From: Thomas Lopez Date: Thu, 28 May 2026 12:11:37 -0400 Subject: [PATCH 5/5] Updating tests and fixing drawshape tool not deleting tmp gameobject when exiting tool --- Editor/EditorCore/DrawShapeTool.cs | 2 +- Tests/Editor/Editor/ShapeToolGridSnapTests.cs | 61 ++++--------------- 2 files changed, 12 insertions(+), 51 deletions(-) diff --git a/Editor/EditorCore/DrawShapeTool.cs b/Editor/EditorCore/DrawShapeTool.cs index e11f2344c..7b532be53 100644 --- a/Editor/EditorCore/DrawShapeTool.cs +++ b/Editor/EditorCore/DrawShapeTool.cs @@ -564,7 +564,7 @@ public override void OnWillBeDeactivated() ProBuilderEditor.selectModeChanged -= OnSelectModeChanged; EditorApplication.playModeStateChanged -= OnPlayModeStateChanged; - if (m_ProBuilderShape != null && !( m_CurrentState is ShapeState_InitShape )) + if (m_ProBuilderShape != null) m_CurrentState = ShapeState.ResetState(); if (m_DuplicateGO != null) diff --git a/Tests/Editor/Editor/ShapeToolGridSnapTests.cs b/Tests/Editor/Editor/ShapeToolGridSnapTests.cs index 3119cbd95..9b38bf9fe 100644 --- a/Tests/Editor/Editor/ShapeToolGridSnapTests.cs +++ b/Tests/Editor/Editor/ShapeToolGridSnapTests.cs @@ -41,6 +41,12 @@ public void DrawShapeTool_GetPoint_SnapsWhenOnGridAndGridSnapActive() var tool = DrawShapeTool.instance; Assume.That(tool, Is.Not.Null); + EditorSnapSettings.gridSnapEnabled = true; + EditorSnapSettings.snapEnabled = true; + + var previousPivotRotation = Tools.pivotRotation; + Tools.pivotRotation = PivotRotation.Global; + // Enable grid mode (this is set by ShapeState_InitShape during hover) tool.m_IsOnGrid = true; @@ -48,17 +54,19 @@ public void DrawShapeTool_GetPoint_SnapsWhenOnGridAndGridSnapActive() if (EditorSnapSettings.gridSnapActive) { // Test position that should snap to grid - Vector3 unsnapedPosition = new Vector3(1.23f, 2.34f, 3.45f); - Vector3 snappedPosition = tool.GetPoint(unsnapedPosition); + Vector3 unsnappedPosition = new Vector3(1.23f, 2.34f, 3.45f); + Vector3 snappedPosition = tool.GetPoint(unsnappedPosition); // Position should be snapped (not equal to original) - Assert.That(snappedPosition, Is.Not.EqualTo(unsnapedPosition)); + Assert.That(snappedPosition, Is.Not.EqualTo(unsnappedPosition)); } else { // Skip test if grid snap is not active Assert.Ignore("Grid snapping is not active in the editor, skipping snap verification"); } + + Tools.pivotRotation = previousPivotRotation; } [Test] @@ -79,17 +87,6 @@ public void DrawShapeTool_GetPoint_DoesNotSnapWhenOffGrid() Assert.That(result, Is.EqualTo(position)); } - [Test] - public void PolyShapeTool_HasGridSnapEnabled() - { - // Test that PolyShapeTool also has gridSnapEnabled (fix from commit 1e25f10d7) - ToolManager.SetActiveTool(); - - // PolyShapeTool doesn't have a static instance like DrawShapeTool, - // so we just verify it was activated successfully - Assert.That(ToolManager.activeToolType, Is.EqualTo(typeof(DrawPolyShapeTool))); - } - [Test] public void DrawShapeTool_GetPoint_RespectsIncrementalSnap() { @@ -112,40 +109,4 @@ public void DrawShapeTool_GetPoint_RespectsIncrementalSnap() Assert.That(snappedPosition.y % snapValue.y, Is.EqualTo(0).Within(0.001f)); Assert.That(snappedPosition.z % snapValue.z, Is.EqualTo(0).Within(0.001f)); } - - /// Test for UUM-141074: - [Test] - public void ShapeState_InitShape_CalculatesGridStateBeforeMouseDown() - { - // This test verifies the fix for UUM-141074 - // Before the fix, m_IsOnGrid was only calculated on MouseDown - // After the fix, it's calculated during hover (before MouseDown) - - ToolManager.SetActiveTool(); - var tool = DrawShapeTool.instance; - Assume.That(tool, Is.Not.Null); - - // The fix ensures that GetPoint can snap correctly during hover - // because m_IsOnGrid is set before GetPoint is called - // We can't directly test the state machine, but we can verify - // that the tool's GetPoint method respects the m_IsOnGrid flag - - // Test 1: When m_IsOnGrid is true, GetPoint should snap (if grid is active) - tool.m_IsOnGrid = true; - Vector3 pos1 = new Vector3(1.23f, 2.34f, 3.45f); - Vector3 result1 = tool.GetPoint(pos1); - - if (EditorSnapSettings.gridSnapActive) - { - Assert.That(result1, Is.Not.EqualTo(pos1), - "GetPoint should snap when m_IsOnGrid is true and grid snap is active"); - } - - // Test 2: When m_IsOnGrid is false, GetPoint should NOT snap - tool.m_IsOnGrid = false; - Vector3 pos2 = new Vector3(1.23f, 2.34f, 3.45f); - Vector3 result2 = tool.GetPoint(pos2); - Assert.That(result2, Is.EqualTo(pos2), - "GetPoint should not snap when m_IsOnGrid is false"); - } }