freezy / VisualPinball.Engine

:video_game: Visual Pinball Engine for Unity
https://docs.visualpinball.org
GNU General Public License v3.0
416 stars 62 forks source link

Bug: Wall editing, point drag failure. #236

Closed Pandelii closed 3 years ago

Pandelii commented 3 years ago

When dragging points on a wall it is possible to produce a failure state by dragging one point directly over another.

Repro steps: Create a wall Enter point edit mode Select the upper left point Drag the red handle (x) to the right until it overlaps the upper right point.

Expected result: Nothing happens or a prompt appears to ask if you want to merge the points. Actual result: The point disappears and the console log indicates a failure.

NullReferenceException: Object reference not set to an instance of an object
VisualPinball.Unity.MeshExtensions.ApplyToUnityMesh (VisualPinball.Engine.VPT.Mesh vpMesh, UnityEngine.Mesh mesh) (at E:/VPE/VisualPinball.Engine-master/VisualPinball.Unity/VisualPinball.Unity/Extensions/MeshExtensions.cs:85)
VisualPinball.Unity.ItemMeshAuthoring`3[TItem,TData,TAuthoring].UpdateMesh () (at E:/VPE/VisualPinball.Engine-master/VisualPinball.Unity/VisualPinball.Unity/VPT/ItemMeshAuthoring.cs:138)
VisualPinball.Unity.ItemMeshAuthoring`3[TItem,TData,TAuthoring].RebuildMeshes () (at E:/VPE/VisualPinball.Engine-master/VisualPinball.Unity/VisualPinball.Unity/VPT/ItemMeshAuthoring.cs:91)
VisualPinball.Unity.ItemMainAuthoring`2[TItem,TData].RebuildMeshIfDirty () (at E:/VPE/VisualPinball.Engine-master/VisualPinball.Unity/VisualPinball.Unity/VPT/ItemMainAuthoring.cs:114)
VisualPinball.Unity.Editor.TransformInspector.RebuildMeshes () (at E:/VPE/VisualPinball.Engine-master/VisualPinball.Unity/VisualPinball.Unity.Editor/VPT/TransformInspector.cs:125)
VisualPinball.Unity.Editor.TransformInspector.OnSceneGUI () (at E:/VPE/VisualPinball.Engine-master/VisualPinball.Unity/VisualPinball.Unity.Editor/VPT/TransformInspector.cs:170)
System.Reflection.MonoMethod.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) (at <9577ac7a62ef43179789031239ba8798>:0)
Rethrow as TargetInvocationException: Exception has been thrown by the target of an invocation.
System.Reflection.MonoMethod.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) (at <9577ac7a62ef43179789031239ba8798>:0)
System.Reflection.MethodBase.Invoke (System.Object obj, System.Object[] parameters) (at <9577ac7a62ef43179789031239ba8798>:0)
UnityEditor.SceneView.CallOnSceneGUI () (at <96da5fbbe3f6429982386da09c8521a8>:0)
UnityEditor.SceneView.HandleSelectionAndOnSceneGUI () (at <96da5fbbe3f6429982386da09c8521a8>:0)
UnityEditor.SceneView.OnGUI () (at <96da5fbbe3f6429982386da09c8521a8>:0)
UnityEditor.HostView.InvokeOnGUI (UnityEngine.Rect onGUIPosition, UnityEngine.Rect viewRect) (at <96da5fbbe3f6429982386da09c8521a8>:0)
UnityEditor.DockArea.DrawView (UnityEngine.Rect viewRect, UnityEngine.Rect dockAreaRect) (at <96da5fbbe3f6429982386da09c8521a8>:0)
UnityEditor.DockArea.OldOnGUI () (at <96da5fbbe3f6429982386da09c8521a8>:0)
UnityEngine.UIElements.IMGUIContainer.DoOnGUI (UnityEngine.Event evt, UnityEngine.Matrix4x4 parentTransform, UnityEngine.Rect clippingRect, System.Boolean isComputingLayout, UnityEngine.Rect layoutSize, System.Action onGUIHandler, System.Boolean canAffectFocus) (at <519636828bf44b44aa4c317f8b947afe>:0)
UnityEngine.GUIUtility:ProcessEvent(Int32, IntPtr, Boolean&)

NullReferenceException: Object reference not set to an instance of an object
VisualPinball.Unity.MeshExtensions.ApplyToUnityMesh (VisualPinball.Engine.VPT.Mesh vpMesh, UnityEngine.Mesh mesh) (at E:/VPE/VisualPinball.Engine-master/VisualPinball.Unity/VisualPinball.Unity/Extensions/MeshExtensions.cs:85)
VisualPinball.Unity.ItemMeshAuthoring`3[TItem,TData,TAuthoring].UpdateMesh () (at E:/VPE/VisualPinball.Engine-master/VisualPinball.Unity/VisualPinball.Unity/VPT/ItemMeshAuthoring.cs:138)
VisualPinball.Unity.ItemMeshAuthoring`3[TItem,TData,TAuthoring].RebuildMeshes () (at E:/VPE/VisualPinball.Engine-master/VisualPinball.Unity/VisualPinball.Unity/VPT/ItemMeshAuthoring.cs:91)
VisualPinball.Unity.ItemMainAuthoring`2[TItem,TData].RebuildMeshIfDirty () (at E:/VPE/VisualPinball.Engine-master/VisualPinball.Unity/VisualPinball.Unity/VPT/ItemMainAuthoring.cs:114)
VisualPinball.Unity.ItemMainAuthoring`2[TItem,TData].OnDrawGizmos () (at E:/VPE/VisualPinball.Engine-master/VisualPinball.Unity/VisualPinball.Unity/VPT/ItemMainAuthoring.cs:178)
UnityEngine.GUIUtility:ProcessEvent(Int32, IntPtr, Boolean&)

PointDragFailure.zip

freezy commented 3 years ago

Thanks for checking this @Vroonsh!

Vroonsh commented 3 years ago

Looked into it, managed to reproduce by crossing the points to create a concave shape image

The surface mesh generator cannot create the top mesh for 2 reasons : Normals could be computed from zero length vector (div by 0) and indices cannot be generated because of the concave shape. Result is a null generated mesh. Dunno if we want to support concave walls, otherwise just checking that the generated mesh is not null to update the UnityMesh is enough to fix the crash, but the result could be ugly (but well...no-one wants to keep an ugly wall on his table ;)) image

Anyway won't be solved by a merge point feature, because it happens without havind drag points close together. But i think the feature could be interesting, but maybe proposed on the drop down menu if there are other drag-points near the selected one.

Pandelii commented 3 years ago

In cases like the second image, Perhaps it should attempt to re-order the points to become a valid shape? For example in that image, the selected point would become 2 and point 2 would become point 3, making a valid shape.

Vroonsh commented 3 years ago

Could become quite complicated to solve :) image But feasable with convex hull. Will need a feedback from the mesh generator to re setup the control points, or just assume we're creating convex hull based on points cloud.