rubber: harden invalid guided state

Author: freezy

VisualPinball.Unity/VisualPinball.Unity.Editor/VPT/Rubber/RubberGuideCommands.cs

@@ -170,13 +170,15 @@ private static void BindSelectedGuidesToRubber(string undoName)
 			if (!rubber || !RubberGuideSlotPickerWindow.TryPick(SelectedGuides(), out var bindings)) {
 				return;
 			}
+			Undo.IncrementCurrentGroup();
 			Undo.RegisterFullObjectHierarchyUndo(rubber.gameObject, undoName);
 			var convertingSpline = rubber.PathSource == RubberPathSource.Spline;
 			string error;
 			var succeeded = convertingSpline
 				? RubberAutofit.TryConvertToGuides(rubber, bindings, out _, out error)
-				: TryReplaceGuideBindings(rubber, bindings, out error);
+				: RubberAutofit.TryReplaceGuideBindings(rubber, bindings, out _, out error);
 			if (!succeeded) {
+				Undo.RevertAllInCurrentGroup();
 				var retainedState = convertingSpline
 					? "The manual spline remains authoritative."
 					: "The bindings were preserved and the previous sampled path was retained.";
@@ -189,13 +191,6 @@ private static void BindSelectedGuidesToRubber(string undoName)
 			RubberGuideDependencyTracker.RebuildSoon();
 		}
 
-		private static bool TryReplaceGuideBindings(RubberComponent rubber,
-			RubberGuideBinding[] bindings, out string error)
-		{
-			rubber.SetGuideBindings(bindings);
-			return RubberAutofit.TryBake(rubber, out _, out error);
-		}
-
 		private static RubberComponent SelectedRubber()
 		{
 			if (Selection.activeGameObject) {

VisualPinball.Unity/VisualPinball.Unity.Editor/VPT/Rubber/RubberInspector.cs

@@ -30,6 +30,7 @@ public class RubberInspector : MainInspector<RubberData, RubberComponent>
 	{
 		private SerializedProperty _thicknessProperty;
 		private SerializedProperty _restLengthProperty;
+		private bool _bindingChangeScheduled;
 
 		protected override void OnEnable()
 		{
@@ -86,13 +87,14 @@ private void DrawPathSection()
 			EditorGUILayout.LabelField("Path", EditorStyles.boldLabel);
 			var source = (RubberPathSource)EditorGUILayout.EnumPopup("Source", MainComponent.PathSource);
 			if (source != MainComponent.PathSource) {
-				Undo.RegisterFullObjectHierarchyUndo(MainComponent.gameObject, "Change Rubber Path Source");
 				if (source == RubberPathSource.Guides) {
-					MainComponent.SetGuideBindings(MainComponent.GuideBindings);
+					EditorUtility.DisplayDialog("Guide Bindings Required",
+						"Select the rubber and its guides, then use GameObject > Pinball > Rubber > Bind Selected Guides.",
+						"OK");
 				} else {
-					MainComponent.DetachFromGuides();
+					ScheduleBindingsChange(MainComponent.GuideBindings.ToArray(),
+						Array.Empty<RubberGuideBinding>(), "Detach Rubber From Guides");
 				}
-				EditorUtility.SetDirty(MainComponent);
 			}
 
 			if (MainComponent.PathSource != RubberPathSource.Guides) {
@@ -101,15 +103,22 @@ private void DrawPathSection()
 				return;
 			}
 
-			var bindings = MainComponent.GuideBindings.ToArray();
+			var expectedBindings = MainComponent.GuideBindings.ToArray();
+			var bindings = expectedBindings.ToArray();
 			for (var i = 0; i < bindings.Length; i++) {
 				EditorGUILayout.BeginVertical(EditorStyles.helpBox);
 				var guide = (RubberGuideComponent)EditorGUILayout.ObjectField($"Guide {i + 1}",
 					bindings[i].Guide, typeof(RubberGuideComponent), true);
 				if (guide != bindings[i].Guide) {
-					bindings[i].Guide = guide;
-					bindings[i].SlotId = default;
-					ApplyBindings(bindings, "Change Rubber Guide Binding");
+					if (!guide) {
+						ScheduleBindingsChange(expectedBindings,
+							bindings.Where((_, index) => index != i).ToArray(),
+							bindings.Length == 1 ? "Detach Rubber From Guides"
+								: "Remove Rubber Guide Binding");
+					} else {
+						ScheduleGuideReplacement(i, expectedBindings, guide);
+					}
+					guide = bindings[i].Guide;
 				}
 				if (guide && guide.Slots.Length > 0) {
 					var selectedSlot = Array.FindIndex(guide.Slots, slot => slot.Id == bindings[i].SlotId);
@@ -122,24 +131,24 @@ private void DrawPathSection()
 						var nextSlot = EditorGUILayout.Popup("Slot", 0, labels);
 						if (nextSlot > 0) {
 							bindings[i].SlotId = guide.Slots[nextSlot - 1].Id;
-							ApplyBindings(bindings, "Repair Rubber Guide Slot");
+							ScheduleBindingsChange(expectedBindings, bindings, "Repair Rubber Guide Slot");
 						}
 					} else {
 						var nextSlot = EditorGUILayout.Popup("Slot", selectedSlot, slotLabels);
 						if (nextSlot != selectedSlot && nextSlot >= 0
 							&& nextSlot < guide.Slots.Length) {
 							bindings[i].SlotId = guide.Slots[nextSlot].Id;
-							ApplyBindings(bindings, "Change Rubber Guide Slot");
+							ScheduleBindingsChange(expectedBindings, bindings, "Change Rubber Guide Slot");
 						}
 					}
 				} else if (guide) {
 					EditorGUILayout.HelpBox("This guide has no slots.", MessageType.Error);
 				}
-				if (GUILayout.Button("Remove Binding")) {
-					ApplyBindings(bindings.Where((_, index) => index != i).ToArray(),
-						"Remove Rubber Guide Binding");
-					EditorGUILayout.EndVertical();
-					break;
+				if (GUILayout.Button(bindings.Length == 1 ? "Detach From Guides" : "Remove Binding")) {
+					ScheduleBindingsChange(expectedBindings,
+						bindings.Where((_, index) => index != i).ToArray(),
+						bindings.Length == 1 ? "Detach Rubber From Guides"
+							: "Remove Rubber Guide Binding");
 				}
 				EditorGUILayout.EndVertical();
 			}
@@ -159,19 +168,109 @@ private void DrawPathSection()
 				}
 			}
 			if (GUILayout.Button("Detach From Guides")) {
-				Undo.RegisterFullObjectHierarchyUndo(MainComponent.gameObject, "Detach Rubber From Guides");
-				MainComponent.DetachFromGuides();
-				MainComponent.RebuildMeshes();
-				EditorUtility.SetDirty(MainComponent);
+				ScheduleBindingsChange(expectedBindings, Array.Empty<RubberGuideBinding>(),
+					"Detach Rubber From Guides");
 			}
 			EditorGUILayout.EndHorizontal();
 		}
 
-		private void ApplyBindings(RubberGuideBinding[] bindings, string undoName)
+		private void ScheduleBindingsChange(RubberGuideBinding[] expectedBindings,
+			RubberGuideBinding[] bindings, string undoName)
+		{
+			if (_bindingChangeScheduled) {
+				return;
+			}
+			expectedBindings = expectedBindings.ToArray();
+			bindings = bindings.ToArray();
+			_bindingChangeScheduled = true;
+			var rubber = MainComponent;
+			EditorApplication.delayCall += () => {
+				if (!this) {
+					return;
+				}
+				_bindingChangeScheduled = false;
+				if (rubber && BindingsMatch(rubber, expectedBindings)) {
+					ApplyBindings(rubber, bindings, undoName);
+				}
+				Repaint();
+			};
+		}
+
+		private void ScheduleGuideReplacement(int bindingIndex, RubberGuideBinding[] expectedBindings,
+			RubberGuideComponent guide)
+		{
+			if (_bindingChangeScheduled) {
+				return;
+			}
+			expectedBindings = expectedBindings.ToArray();
+			_bindingChangeScheduled = true;
+			var rubber = MainComponent;
+			EditorApplication.delayCall += () => {
+				if (!this) {
+					return;
+				}
+				_bindingChangeScheduled = false;
+				if (!rubber || !guide || bindingIndex < 0
+					|| bindingIndex >= rubber.GuideBindings.Count) {
+					Repaint();
+					return;
+				}
+				if (!BindingsMatch(rubber, expectedBindings)) {
+					Repaint();
+					return;
+				}
+				if (RubberGuideSlotPickerWindow.TryPick(new[] { guide }, out var selectedBindings)
+					&& BindingsMatch(rubber, expectedBindings)) {
+					var bindings = rubber.GuideBindings.ToArray();
+					bindings[bindingIndex] = selectedBindings[0];
+					ApplyBindings(rubber, bindings, "Change Rubber Guide Binding");
+				}
+				Repaint();
+			};
+		}
+
+		private static bool BindingsMatch(RubberComponent rubber, RubberGuideBinding[] expectedBindings)
+		{
+			if (rubber.PathSource != RubberPathSource.Guides
+				|| rubber.GuideBindings.Count != expectedBindings.Length) {
+				return false;
+			}
+			for (var i = 0; i < expectedBindings.Length; i++) {
+				var current = rubber.GuideBindings[i];
+				var expected = expectedBindings[i];
+				if (current.Guide != expected.Guide || current.SlotId != expected.SlotId) {
+					return false;
+				}
+			}
+			return true;
+		}
+
+		private static void ApplyBindings(RubberComponent rubber, RubberGuideBinding[] bindings, string undoName)
 		{
-			Undo.RegisterFullObjectHierarchyUndo(MainComponent.gameObject, undoName);
-			MainComponent.SetGuideBindings(bindings);
-			EditorUtility.SetDirty(MainComponent);
+			Undo.IncrementCurrentGroup();
+			Undo.RegisterFullObjectHierarchyUndo(rubber.gameObject, undoName);
+			var detaching = bindings.Length == 0;
+			if (detaching) {
+				rubber.DetachFromGuides();
+				rubber.RebuildMeshes();
+			} else if (!RubberAutofit.TryReplaceGuideBindings(rubber, bindings,
+				out _, out var error)) {
+				Undo.RevertAllInCurrentGroup();
+				EditorUtility.DisplayDialog("Rubber Binding Change Failed",
+					$"The previous bindings and sampled path were preserved.\n\n{error}", "OK");
+				return;
+			}
+			EditorUtility.SetDirty(rubber);
+			PrefabUtility.RecordPrefabInstancePropertyModifications(rubber);
+			if (detaching) {
+				var collider = rubber.GetComponent<RubberColliderComponent>();
+				if (collider) {
+					EditorUtility.SetDirty(collider);
+					PrefabUtility.RecordPrefabInstancePropertyModifications(collider);
+				}
+			}
+			RubberGuideDependencyTracker.RebuildSoon();
+			SceneView.RepaintAll();
 		}
 
 		private void OnSceneGUI()

VisualPinball.Unity/VisualPinball.Unity.Test/Physics/SweptCircleColliderTests.cs

@@ -11,6 +11,8 @@
 using Unity.Collections;
 using Unity.Mathematics;
 using UnityEngine;
+using UnityEngine.TestTools;
+using VisualPinball.Engine.Math;
 
 namespace VisualPinball.Unity.Test
 {
@@ -256,6 +258,38 @@ public void ThinCordShouldUseRadiusRelativeChordTolerance()
 					.Within(1e-6f));
 		}
 
+		[Test]
+		public void InvalidPhysicalRubberShouldGenerateLegacyFallbackColliders()
+		{
+			var go = new GameObject("Rubber");
+			go.SetActive(false);
+			var transforms = new NativeParallelHashMap<int, float4x4>(1, Allocator.Temp);
+			var references = new ColliderReference(ref transforms, Allocator.Temp);
+			try {
+				var rubber = go.AddComponent<RubberComponent>();
+				rubber.DragPoints = new[] {
+					new DragPointData(-10f, -10f),
+					new DragPointData(-10f, 10f),
+					new DragPointData(10f, 10f),
+					new DragPointData(10f, -10f),
+				};
+				var collider = go.AddComponent<RubberColliderComponent>();
+				collider.Mode = RubberColliderMode.Physical;
+				var api = new TestRubberApi(go);
+				LogAssert.Expect(LogType.Warning,
+					"Physical rubber 'Rubber' has no current valid guided bake; falling back to Legacy collision.");
+
+				api.GenerateColliders(ref references);
+
+				Assert.That(references.Count, Is.GreaterThan(0));
+				Assert.That(references.SweptCircleColliders.Length, Is.Zero);
+			} finally {
+				references.Dispose();
+				transforms.Dispose();
+				UnityEngine.Object.DestroyImmediate(go);
+			}
+		}
+
 		[Test]
 		public void PhysicalRubberGeneratorShouldCreateContinuousRoundCordSegments()
 		{
@@ -344,6 +378,16 @@ public void PhysicalRubberGeneratorShouldApplyOffsetAlongBakeNormal()
 			}
 		}
 
+		private sealed class TestRubberApi : RubberApi
+		{
+			internal TestRubberApi(GameObject go) : base(go, null, null)
+			{
+			}
+
+			internal void GenerateColliders(ref ColliderReference references)
+				=> CreateColliders(ref references, float4x4.identity, 0f);
+		}
+
 		private static SweptCircleCollider CreateCollider()
 		{
 			return new SweptCircleCollider(new float3(-10f, 0f, 0f),

VisualPinball.Unity/VisualPinball.Unity.Test/VPT/RubberAutofitTests.cs

@@ -215,6 +215,99 @@ public void FailedRebindShouldRetainLastBakedGeometry()
 			}
 		}
 
+		[Test]
+		public void FailedGuidedReplacementShouldRestoreCurrentBake()
+		{
+			var rubberObject = new GameObject("Rubber");
+			var guideObject = new GameObject("Guide");
+			try {
+				var rubber = rubberObject.AddComponent<RubberComponent>();
+				var guide = AddGuide(guideObject, 0.01f);
+				var binding = new RubberGuideBinding(guide, guide.Slots[0].Id);
+				rubber.SetGuideBindings(new[] { binding });
+				Assert.That(RubberAutofit.TryBake(rubber, out _, out var bakeError),
+					Is.True, bakeError);
+				var previousPath = rubber.BakedPath.ToArray();
+				var previousVersion = rubber.BakeVersion;
+				var previousHash = rubber.BakeInputHash;
+				var previousFrame = rubber.BakeFrameToLocal;
+
+				var replaced = RubberAutofit.TryReplaceGuideBindings(rubber, new[] {
+					new RubberGuideBinding(guide, SerializedGuid.New()),
+				}, out _, out var error);
+
+				Assert.That(replaced, Is.False);
+				Assert.That(error, Does.Contain("missing slot"));
+				Assert.That(rubber.PathSource, Is.EqualTo(RubberPathSource.Guides));
+				Assert.That(rubber.GuideBindings.Count, Is.EqualTo(1));
+				Assert.That(rubber.GuideBindings[0].Guide, Is.SameAs(binding.Guide));
+				Assert.That(rubber.GuideBindings[0].SlotId, Is.EqualTo(binding.SlotId));
+				Assert.That(rubber.BakedPath, Is.EqualTo(previousPath));
+				Assert.That(rubber.BakeVersion, Is.EqualTo(previousVersion));
+				Assert.That(rubber.BakeInputHash, Is.EqualTo(previousHash));
+				Assert.That(rubber.BakeFrameToLocal, Is.EqualTo(previousFrame));
+				Assert.That(RubberAutofit.GetStatus(rubber).IsValid, Is.True);
+			} finally {
+				UnityEngine.Object.DestroyImmediate(rubberObject);
+				UnityEngine.Object.DestroyImmediate(guideObject);
+			}
+		}
+
+		[Test]
+		public void GuidedReplacementShouldRejectSplineRubberWithoutMutation()
+		{
+			var rubberObject = new GameObject("Rubber");
+			var guideObject = new GameObject("Guide");
+			try {
+				var rubber = rubberObject.AddComponent<RubberComponent>();
+				var guide = AddGuide(guideObject, 0.01f);
+
+				var replaced = RubberAutofit.TryReplaceGuideBindings(rubber, new[] {
+					new RubberGuideBinding(guide, guide.Slots[0].Id),
+				}, out _, out var error);
+
+				Assert.That(replaced, Is.False);
+				Assert.That(error, Does.Contain("guided rubber"));
+				Assert.That(rubber.PathSource, Is.EqualTo(RubberPathSource.Spline));
+				Assert.That(rubber.GuideBindings, Is.Empty);
+				Assert.That(rubber.BakedPath, Is.Empty);
+			} finally {
+				UnityEngine.Object.DestroyImmediate(rubberObject);
+				UnityEngine.Object.DestroyImmediate(guideObject);
+			}
+		}
+
+		[Test]
+		public void DetachingGuidedRubberShouldKeepSampledSpline()
+		{
+			var rubberObject = new GameObject("Rubber");
+			var guideObject = new GameObject("Guide");
+			try {
+				var rubber = rubberObject.AddComponent<RubberComponent>();
+				var collider = rubberObject.AddComponent<RubberColliderComponent>();
+				collider.Mode = RubberColliderMode.Physical;
+				var guide = AddGuide(guideObject, 0.01f);
+				rubber.SetGuideBindings(new[] {
+					new RubberGuideBinding(guide, guide.Slots[0].Id),
+				});
+				Assert.That(RubberAutofit.TryBake(rubber, out _, out var bakeError),
+					Is.True, bakeError);
+				var sampledPoints = rubber.DragPoints.Select(point => point.Center).ToArray();
+
+				rubber.DetachFromGuides();
+				rubber.RebuildMeshes();
+
+				Assert.That(rubber.PathSource, Is.EqualTo(RubberPathSource.Spline));
+				Assert.That(rubber.GuideBindings, Is.Empty);
+				Assert.That(rubber.BakedPath, Is.Empty);
+				Assert.That(rubber.DragPoints.Select(point => point.Center), Is.EqualTo(sampledPoints));
+				Assert.That(collider.Mode, Is.EqualTo(RubberColliderMode.Legacy));
+			} finally {
+				UnityEngine.Object.DestroyImmediate(rubberObject);
+				UnityEngine.Object.DestroyImmediate(guideObject);
+			}
+		}
+
 		[Test]
 		public void FailedSplineConversionShouldRestoreManualAuthority()
 		{