diff --git a/Components/Components/Containers/ChiselModelComponent.cs b/Components/Components/Containers/ChiselModelComponent.cs index 4b321d5a..cb76c97d 100644 --- a/Components/Components/Containers/ChiselModelComponent.cs +++ b/Components/Components/Containers/ChiselModelComponent.cs @@ -278,23 +278,20 @@ public override void GetMessages(IChiselMessageHandler messages) { // TODO: improve warning messages const string kModelHasNoChildrenMessage = kNodeTypeName + " has no children and will not have an effect"; - const string kFailedToGenerateNodeMessage = "Failed to generate internal representation of " + kNodeTypeName + "\nThe model might not have any (active) brushes inside it."; + const string kFailedToGenerateNodeMessage = "Failed to generate internal representation of " + kNodeTypeName + " (this should never happen)"; //const string kChildNodesWithProblems = kNodeTypeName + " has children with errors"; if (!Node.Valid) messages.Warning(kFailedToGenerateNodeMessage); // A model makes no sense without any children + if (hierarchyItem != null && hierarchyItem.Registered) + { + if (hierarchyItem.Children.Count == 0) + messages.Warning(kModelHasNoChildrenMessage); + } else if (transform.childCount == 0) - { messages.Warning(kModelHasNoChildrenMessage); - } else - { - if (transform.GetComponentInChildren() == null) - { - messages.Warning(kModelHasNoChildrenMessage); - } - } //if (invalidChildNodes.Count > 0) // messages.Warning(kChildNodesWithProblems); diff --git a/Components/Managers/ChiselNodeHierarchyManager.cs b/Components/Managers/ChiselNodeHierarchyManager.cs index 18c63248..1f3c6eb5 100644 --- a/Components/Managers/ChiselNodeHierarchyManager.cs +++ b/Components/Managers/ChiselNodeHierarchyManager.cs @@ -1476,8 +1476,12 @@ internal static void UpdateTrampoline() { sceneHierarchy.RootItems.Add(hierarchyItem); } - + sortChildrenQueue.Add(sceneHierarchy.RootItems); + + // a model without children gets its tree too, nothing else queues it + if (hierarchyItem.Component.IsContainer) + updateChildrenQueue.Add(hierarchyItem); } } addToHierarchyQueue.Clear(); diff --git a/Components/Tests/ChiselHierarchyEditTests.cs b/Components/Tests/ChiselHierarchyEditTests.cs index aa094cb3..0b211941 100644 --- a/Components/Tests/ChiselHierarchyEditTests.cs +++ b/Components/Tests/ChiselHierarchyEditTests.cs @@ -114,6 +114,39 @@ static string TreeOrder(ChiselModelComponent model) return names.ToString(); } + class Messages : Chisel.Core.IChiselMessageHandler + { + public readonly List warnings = new(); + public Chisel.Core.MessageDestination Destination => Chisel.Core.MessageDestination.Inspector; + public void SetTitle(string name, Object reference) { } + public void Warning(string message, System.Action buttonAction, string buttonText) { warnings.Add(message); } + public void Warning(string message) { warnings.Add(message); } + } + + [Test] + public void ANewEmptyModel_HasItsTree_AndSaysItHasNoChildren() + { + var model = Model("empty"); + Update(); + Assert.That(model.Node.Valid, Is.True); + var messages = new Messages(); + model.GetMessages(messages); + Assert.That(messages.warnings, Has.Count.EqualTo(1)); + Assert.That(messages.warnings[0], Does.Contain("has no children")); + + var box = Box("box", model.transform, Vector3.zero); + Update(); + messages.warnings.Clear(); + model.GetMessages(messages); + Assert.That(messages.warnings, Is.Empty); + + Object.DestroyImmediate(box); + Update(); + model.GetMessages(messages); + Assert.That(messages.warnings, Has.Count.EqualTo(1), "its brush deleted"); + Assert.That(messages.warnings[0], Does.Contain("has no children")); + } + [Test] public void DeletingABrush_RebuildsTheBrushItTouched() { diff --git a/Editor/ComponentEditors/Base/ChiselNodeEditor.cs b/Editor/ComponentEditors/Base/ChiselNodeEditor.cs index 7494b438..a3cb5518 100644 --- a/Editor/ComponentEditors/Base/ChiselNodeEditor.cs +++ b/Editor/ComponentEditors/Base/ChiselNodeEditor.cs @@ -838,13 +838,11 @@ protected override void OnEditSettingsGUI(SceneView sceneView) readonly static ChiselComponentInspectorMessageHandler s_Warnings = new(); - Vector2 messagesScrollPosition = Vector2.zero; - protected virtual void OnInspector() { - s_Warnings.StartWarnings(messagesScrollPosition); + s_Warnings.StartWarnings(); ChiselMessages.ShowMessages(targets, s_Warnings); - messagesScrollPosition = s_Warnings.EndWarnings(); + s_Warnings.EndWarnings(); OnDefaultInspector(); } diff --git a/Editor/ComponentEditors/Containers/ChiselModelEditor.cs b/Editor/ComponentEditors/Containers/ChiselModelEditor.cs index f066c08d..ca8c3e01 100644 --- a/Editor/ComponentEditors/Containers/ChiselModelEditor.cs +++ b/Editor/ComponentEditors/Containers/ChiselModelEditor.cs @@ -172,7 +172,6 @@ internal static bool ValidateActiveModel(MenuCommand menuCommand) readonly static ChiselComponentInspectorMessageHandler s_Messages = new(); //private Vector2 modelMessagesScrollPosition = Vector2.zero; - Vector2 childMessagesScrollPosition = Vector2.zero; delegate float GetCachedMeshSurfaceAreaDelegate(MeshRenderer meshRenderer); @@ -1033,9 +1032,9 @@ public override void OnInspectorGUI() if (targets.Length == 1) { - s_Messages.StartWarnings(childMessagesScrollPosition); + s_Messages.StartWarnings(); ChiselMessages.ShowMessages(childNodes, s_Messages); - childMessagesScrollPosition = s_Messages.EndWarnings(); + s_Messages.EndWarnings(); } EditorGUI.BeginChangeCheck(); diff --git a/Editor/SceneView/ChiselEditorHandles.cs b/Editor/SceneView/ChiselEditorHandles.cs index 59aa4023..9698bf5b 100644 --- a/Editor/SceneView/ChiselEditorHandles.cs +++ b/Editor/SceneView/ChiselEditorHandles.cs @@ -1456,180 +1456,75 @@ public bool DoPlanarScaleHandle(ref Vector2 scale2D, Vector3 position, Quaternio } } + // Collects a node's messages while the inspector asks for them, and shows each once as a help box public sealed class ChiselComponentInspectorMessageHandler : IChiselMessageHandler { - public MessageDestination Destination + struct Entry { - get { return MessageDestination.Inspector; } + public string title; + public UnityEngine.Object reference; + public string message; + public Action buttonAction; + public string buttonText; } + readonly List entries = new(); + readonly HashSet<(UnityEngine.Object, string)> shown = new(); + string titleName; + UnityEngine.Object titleReference; - bool hasLayoutStarted = false; - public void StartWarnings(Vector2 position) - { - hasLayoutStarted = false; - s_ScrollPosition = position; - } - - static GUIContent s_WarningIcon; - static GUIContent s_WarningIcon2x; - static Vector2 s_IconSize; - static Vector2 s_IconSize2x; - static Vector2 s_ScrollPosition; - - int indentLevel; - float height; + public MessageDestination Destination => MessageDestination.Inspector; + public void StartWarnings() + { + entries.Clear(); + shown.Clear(); + titleName = null; + titleReference = null; + } - string titleName = null; - bool titleShown = false; - UnityEngine.Object titleReferenceObject; public void SetTitle(string name, UnityEngine.Object reference) { titleName = name; - titleShown = false; - titleReferenceObject = reference; - } + titleReference = reference; + } - void StartWarningLayout() + public void Warning(string message, Action buttonAction, string buttonText) { - if (hasLayoutStarted) + if (string.IsNullOrEmpty(message) || !shown.Add((titleReference, message))) return; - if (s_WarningIcon == null) - { - s_WarningIcon = EditorGUIUtility.IconContent("console.warnicon.sml"); - s_WarningIcon2x = EditorGUIUtility.IconContent("console.warnicon.sml@2x"); - GUIStyle defaultStyle = EditorStyles.helpBox; - s_IconSize = defaultStyle.CalcSize(s_WarningIcon); - s_IconSize2x = defaultStyle.CalcSize(s_WarningIcon2x); - } + entries.Add(new Entry { title = titleName, reference = titleReference, message = message, buttonAction = buttonAction, buttonText = buttonText }); + } - indentLevel = EditorGUI.indentLevel; - EditorGUI.indentLevel = 0; + public void Warning(string message) { Warning(message, null, null); } - s_ScrollPosition = GUILayout.BeginScrollView(s_ScrollPosition, EditorStyles.helpBox, GUILayout.ExpandWidth(true), GUILayout.MaxHeight(150), GUILayout.MinHeight(0)); - hasLayoutStarted = true; - height = 0; - } - - public (bool, float) ShowWarningIcon() + public void EndWarnings() { - Rect rect = GUILayoutUtility.GetRect(0, 0, EditorStyles.helpBox); - if (EditorGUIUtility.pixelsPerPoint > 1) - { - rect.width = s_IconSize2x.x; - rect.height = s_IconSize2x.y; - EditorGUI.LabelField(rect, s_WarningIcon2x); - return (true, rect.yMax); - } else + var indentLevel = EditorGUI.indentLevel; + EditorGUI.indentLevel = 0; + UnityEngine.Object lastReference = null; + foreach (var entry in entries) { - rect.width = s_IconSize.x; - rect.height = s_IconSize.y; - EditorGUI.LabelField(rect, s_WarningIcon); - return (false, rect.yMax); - } - } - - public Vector2 EndWarnings() - { - if (hasLayoutStarted) - { - if (EditorGUIUtility.pixelsPerPoint > 1) + if (!string.IsNullOrWhiteSpace(entry.title) && entry.reference != lastReference) { - if (height < s_IconSize2x.y) - GUILayoutUtility.GetRect(0, (s_IconSize2x.y - height) - EditorGUIUtility.singleLineHeight); - } else - { - if (height < s_IconSize.y) - GUILayoutUtility.GetRect(0, (s_IconSize.y - height) - EditorGUIUtility.singleLineHeight); + if (GUILayout.Button(entry.title, EditorStyles.miniBoldLabel) && entry.reference) + EditorGUIUtility.PingObject(entry.reference); + lastReference = entry.reference; } - EditorGUI.indentLevel = indentLevel; - GUILayout.EndScrollView(); - hasLayoutStarted = false; - titleShown = false; - titleName = null; - } - return s_ScrollPosition; - } - - public void ShowTitle() - { - if (titleShown) - return; - EditorGUI.indentLevel = 0; - (bool doubleSized, float warningIconBottom) = ShowWarningIcon(); - if (!string.IsNullOrWhiteSpace(titleName)) - { - var content = new GUIContent(titleName); - Rect rect = GUILayoutUtility.GetRect(content, EditorStyles.label); - if (doubleSized) - { - rect.x += s_IconSize2x.x; - rect.width -= s_IconSize2x.x; - height = math.max(rect.yMax, warningIconBottom) + 3; - } else - { - rect.x += s_IconSize.x; - rect.width -= s_IconSize.x; - height = math.max(rect.yMax, warningIconBottom) + 3; - } - if (titleReferenceObject == null) - { - EditorGUI.LabelField(rect, content, EditorStyles.label); - } else + if (entry.buttonAction == null) { - if (GUI.Button(rect, content, EditorStyles.label)) - { - EditorGUIUtility.PingObject(titleReferenceObject); - } + EditorGUILayout.HelpBox(entry.message, MessageType.Warning); + continue; } - EditorGUI.indentLevel = 1; - } - titleShown = true; - } - - public void Warning(string message, Action buttonAction, string buttonText) - { - if (!hasLayoutStarted) - StartWarningLayout(); - if (!titleShown) - ShowTitle(); - - // TODO: prevent duplicates? - EditorGUILayout.BeginHorizontal(); - try - { - var content = new GUIContent(message); - Rect rect = GUILayoutUtility.GetRect(content, EditorStyles.label); - rect.x += s_IconSize.x; - rect.width -= s_IconSize.x; - EditorGUI.LabelField(rect, content); - var content2 = new GUIContent(buttonText); - Rect rect2 = GUILayoutUtility.GetRect(content2, EditorStyles.iconButton); - if (GUI.Button(rect2, content2, EditorStyles.iconButton)) - buttonAction?.Invoke(); - height += rect.height + rect2.height; - } - finally - { + EditorGUILayout.BeginHorizontal(); + EditorGUILayout.HelpBox(entry.message, MessageType.Warning); + if (GUILayout.Button(entry.buttonText, GUILayout.ExpandWidth(false), GUILayout.ExpandHeight(true))) + entry.buttonAction(); EditorGUILayout.EndHorizontal(); } - } - - public void Warning(string message) - { - if (!hasLayoutStarted) - StartWarningLayout(); - if (!titleShown) - ShowTitle(); - - // TODO: prevent duplicates? - var content = new GUIContent(message); - Rect rect = GUILayoutUtility.GetRect(content, EditorStyles.label); - rect.x += s_IconSize.x; - rect.width -= s_IconSize.x; - EditorGUI.LabelField(rect, content); - height += rect.height; - } + EditorGUI.indentLevel = indentLevel; + entries.Clear(); + shown.Clear(); + } } } diff --git a/Editor/SceneView/SceneHandles/SceneHandles.Slider1D.cs b/Editor/SceneView/SceneHandles/SceneHandles.Slider1D.cs index 25d25a5f..e7088f13 100644 --- a/Editor/SceneView/SceneHandles/SceneHandles.Slider1D.cs +++ b/Editor/SceneView/SceneHandles/SceneHandles.Slider1D.cs @@ -181,7 +181,7 @@ internal static Vector3[] Do(int id, Axis axis, Vector3[] points, int pointCount handleMatrix.MultiplyPoint(handleOrigin), handleMatrix.MultiplyVector(slideDirection), snappingStep, axis); - s_Snapping1D.CalculateExtents(SceneHandles.InverseMatrix, s_StartPoints); + s_Snapping1D.CalculateExtents(handleMatrix, s_StartPoints); s_MovedMouse = false; break; } diff --git a/Editor/SceneView/Snapping/SceneHandles.Snapping.cs b/Editor/SceneView/Snapping/SceneHandles.Snapping.cs index af2f265b..5cd1510d 100644 --- a/Editor/SceneView/Snapping/SceneHandles.Snapping.cs +++ b/Editor/SceneView/Snapping/SceneHandles.Snapping.cs @@ -714,11 +714,11 @@ public class Snapping1D public void Initialize(Vector2 currentMousePosition, Vector3 slideStart, Vector3 slideDirection, float snappingStep, Axis axis) { - this.slideDirection = slideDirection; + this.slideDirection = slideDirection.normalized; this.snappingStep = snappingStep; this.slideStart = slideStart; - this.slideOrigin = SceneHandleUtility.ProjectPointRay(Grid.ActiveGrid.Center, slideStart, slideDirection); + this.slideOrigin = SceneHandleUtility.ProjectPointRay(Grid.ActiveGrid.Center, slideStart, this.slideDirection); this.slideExtents.min = this.slideExtents.max = 0; @@ -726,26 +726,24 @@ public void Initialize(Vector2 currentMousePosition, Vector3 slideStart, Vector3 this.slidePosition = this.slideOrigin; this.slideOffset = slideStart - this.slideOrigin; - this.startOffset = SnappingUtility.WorldPointToDistance(slideStart - this.slidePosition, slideDirection); + this.startOffset = SnappingUtility.WorldPointToDistance(slideStart - this.slidePosition, this.slideDirection); this.startMousePosition = currentMousePosition; this.slideAxis = axis; this.snapResult = SnapResult1D.None; - this.min = slideStart + SnappingUtility.DistanceToWorldPoint (slideExtents.min, slideDirection); - this.max = slideStart + SnappingUtility.DistanceToWorldPoint (slideExtents.max, slideDirection); + this.min = slideStart + SnappingUtility.DistanceToWorldPoint (slideExtents.min, this.slideDirection); + this.max = slideStart + SnappingUtility.DistanceToWorldPoint (slideExtents.max, this.slideDirection); } - static Extents1D GetExtentsOfPointArray(Matrix4x4 matrix, Vector3[] points, Vector3 slideOrigin, Vector3 slideDirection) + // Distances along the line in world space, so a scaled handle matrix doesn't scale them + static Extents1D GetExtentsOfPointArray(Matrix4x4 localToWorld, Vector3[] points, Vector3 slideOrigin, Vector3 slideDirection) { - var transformedDirection = matrix.MultiplyVector(slideDirection); - var transformedOrigin = matrix.MultiplyPoint(slideOrigin); - var min = float.PositiveInfinity; var max = float.NegativeInfinity; for (var i = 0; i < points.Length; i++) { - var distance = SnappingUtility.WorldPointToDistance(points[i], transformedDirection, transformedOrigin); + var distance = SnappingUtility.WorldPointToDistance(localToWorld.MultiplyPoint(points[i]), slideDirection, slideOrigin); min = Mathf.Min(min, distance); max = Mathf.Max(max, distance); } diff --git a/Editor/Tests/ChiselSnapping1DTests.cs b/Editor/Tests/ChiselSnapping1DTests.cs new file mode 100644 index 00000000..9f01a56b --- /dev/null +++ b/Editor/Tests/ChiselSnapping1DTests.cs @@ -0,0 +1,28 @@ +using Chisel.Core; +using NUnit.Framework; +using UnityEngine; + +namespace Chisel.Editors.Tests +{ + [TestFixture] + public class ChiselSnapping1DTests + { + // A side handle of a brush scaled along x: the extents it snaps are world distances along the line + [Test] + public void TheExtentsOfAScaledHandle_AreWorldDistances() + { + var localToWorld = Matrix4x4.Scale(new Vector3(2, 1, 1)); + var localPoint = new Vector3(0.5f, 0, 0); + var worldPoint = localToWorld.MultiplyPoint(localPoint); + + var snapping = new Snapping1D(); + snapping.Initialize(Vector2.zero, worldPoint, localToWorld.MultiplyVector(Vector3.right), 1, Axis.X); + snapping.CalculateExtents(localToWorld, new[] { localPoint }); + + var origin = Grid.ActiveGrid.Center; + var expected = worldPoint.x - origin.x; + Assert.That(snapping.WorldSnappedExtents.min, Is.EqualTo(expected).Within(1e-5f)); + Assert.That(snapping.WorldSnappedExtents.max, Is.EqualTo(expected).Within(1e-5f)); + } + } +} diff --git a/Editor/Tests/ChiselSnapping1DTests.cs.meta b/Editor/Tests/ChiselSnapping1DTests.cs.meta new file mode 100644 index 00000000..f4dc3b0c --- /dev/null +++ b/Editor/Tests/ChiselSnapping1DTests.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 1306168fc35491145929f627032dede4 \ No newline at end of file