Merge pull request #583 from Unity-Technologies/bugfix/pbld-205-polyshape-merging

Fix for Edit PolyShape tool reverting previously merged polyshapes

Author: modrimkus-unity

CHANGELOG.md

@@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - [STO-3432] Fixed a bug where the Polyshape creation tool was not placing the first point on a custom grids correctly.
 - [PBLD-183] Fixed a bug where the **Extrude by** setting of the **Extrude Faces** action was always set to **Individual Faces**.
 - [STO-3442] Fixed a bug where hover-highlighted elements are not always selected.
+- [PBLD-205] Fixed a bug where the `Edit PolyShape` tool would revert previously merged PolyShape objects.
 
 ## [6.0.4] - 2024-09-12
 

Editor/MenuActions/Object/MergeObjects.cs

@@ -6,6 +6,7 @@
 using UnityEditor;
 using UnityEditor.ProBuilder.UI;
 using UnityEngine.ProBuilder.MeshOperations;
+using UnityEngine.ProBuilder.Shapes;
 
 namespace UnityEditor.ProBuilder.Actions
 {
@@ -39,6 +40,12 @@ protected override ActionResult PerformActionImplementation()
             if (MeshSelection.selectedObjectCount < 2)
                 return new ActionResult(ActionResult.Status.Canceled, "Must Select 2+ Objects");
 
+            DoMergeObjectsAction();
+            return new ActionResult(ActionResult.Status.Success, "Merged Objects");
+        }
+
+        internal List<ProBuilderMesh> DoMergeObjectsAction()
+        {
             var selected = MeshSelection.top.ToArray();
             ProBuilderMesh currentMesh = MeshSelection.activeMesh;
             UndoUtility.RecordObject(currentMesh, "Merge Objects");
@@ -53,8 +60,18 @@ protected override ActionResult PerformActionImplementation()
                     {
                         mesh.gameObject.name = Selection.activeGameObject.name + "-Merged";
                         UndoUtility.RegisterCreatedObjectUndo(mesh.gameObject, "Merge Objects");
+                        
                         Selection.objects = res.Select(x => x.gameObject).ToArray();
                     }
+                    
+                    // Remove PolyShape and ProBuilderShape components if any are present post-merge
+                    var polyShapeComp = mesh.gameObject.GetComponent<PolyShape>();
+                    if (polyShapeComp != null )
+                        UndoUtility.DestroyImmediate(polyShapeComp);
+                    
+                    var proBuilderShape = mesh.gameObject.GetComponent<ProBuilderShape>();
+                    if (proBuilderShape != null )
+                        UndoUtility.DestroyImmediate(proBuilderShape);
                 }
 
                 // Delete donor objects if they are not part of the result
@@ -65,8 +82,7 @@ protected override ActionResult PerformActionImplementation()
                 }
             }
 
-            ProBuilderEditor.Refresh();
-            return new ActionResult(ActionResult.Status.Success, "Merged Objects");
+            return res;
         }
     }
 }

Tests/Editor/Actions/MergeObjectsTest.cs

@@ -0,0 +1,67 @@
+using System.Collections.Generic;
+using UObject = UnityEngine.Object;
+using UnityEngine;
+using NUnit.Framework;
+using UnityEditor;
+using UnityEditor.ProBuilder;
+using UnityEditor.ProBuilder.Actions;
+using UnityEngine.ProBuilder;
+using UnityEngine.ProBuilder.Shapes;
+
+public class MergeObjectsTest 
+{
+    ProBuilderMesh m_mesh1;
+    ProBuilderMesh m_mesh2;
+    
+    [SetUp]
+    public void SetUp()
+    {
+        m_mesh1 = ShapeFactory.Instantiate<Cube>();
+        m_mesh2 = ShapeFactory.Instantiate<Cone>();
+    }
+
+    [TearDown]
+    public void Cleanup()
+    {
+        UObject.DestroyImmediate(m_mesh1);
+        UObject.DestroyImmediate(m_mesh2);
+    }
+    
+    [Test]
+    [TestCase(typeof(PolyShape),typeof(PolyShape))] 
+    [TestCase(typeof(ProBuilderShape),typeof(ProBuilderShape))] 
+    [TestCase(typeof(ProBuilderShape),typeof(PolyShape))]
+    public void MergeObjects_WithShapeCompPresent_ResultNoLongerHasShapeComp(System.Type shapeCompTypeA, System.Type shapeCompTypeB)
+    {
+        void AttachComponentOfType(ProBuilderMesh mesh, System.Type shapeCompType)
+        {
+            if (shapeCompType == typeof(PolyShape)) 
+                mesh.gameObject.AddComponent<PolyShape>();
+            else
+                mesh.gameObject.AddComponent<ProBuilderShape>();
+        }
+        
+        var meshes = new List<ProBuilderMesh>();
+        meshes.Add(m_mesh1);
+        meshes.Add(m_mesh2);
+        
+        Assume.That(meshes.Count, Is.EqualTo(2));
+
+        AttachComponentOfType(meshes[0], shapeCompTypeA);
+        AttachComponentOfType(meshes[1], shapeCompTypeB);
+        
+        MeshSelection.SetSelection(new List<GameObject>( new[]{ m_mesh1.gameObject, m_mesh2.gameObject }));
+        ActiveEditorTracker.sharedTracker.ForceRebuild();
+
+        var mergeObjectsAction = new MergeObjects();
+        var newMeshes = mergeObjectsAction.DoMergeObjectsAction();
+        
+        Assume.That(newMeshes.Count, Is.EqualTo(1), "There should only be one mesh after merging objects.");
+
+        var shapeCompFound = false; 
+        shapeCompFound |= newMeshes[0].gameObject.GetComponent<PolyShape>() != null;
+        shapeCompFound |= newMeshes[0].gameObject.GetComponent<ProBuilderShape>() != null;
+        
+        Assert.That(shapeCompFound, Is.Not.True, $"There should be no {shapeCompTypeA.Name} or {shapeCompTypeB.Name} component on ProBuilder Meshes after mesh combine.");
+    }
+}