[UUM-133530] Ensure "Set Double Sided" is enabled when active selection is proper (#660)

* Fix the 'Set Double Sided' action in the Editor sample. It couldn't be enabled in the contextual menu due to a missing override.

* Add changelog entry

* Ensure to pass the current status of a MenuAction when there is no options, nor fileMenuEntry associated to it

* Add tests to cover some cases of the contextual menu

* Update changelog

* Clean up

* Remove unnecessary comment

* Refactor

* Fix changelog after merge

* Clean up documentation in CustomAction.cs

Removed empty return documentation from methods.

Author: thomas-tu

CHANGELOG.md

@@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ### Fixed
 
 - [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-133530] Fixed the `Set Double Sided` custom action in the Editor Sample, which was previously remaining disabled.
+- [UUM-133530] Ensured that the context menu respects the value of `MenuAction.enabled`.
 - [UUM-133531] Fixed component icons in Light theme.
 - [UUM-133526] Material Editor: fixed a warning (`GUI Error: Invalid GUILayout state in MaterialEditor view.`) that was thrown when deleting an extra material slot.
 - Fixed warnings related to obsolete API calls with Unity 6.4 and onwards.

Editor/EditorCore/ProBuilderToolsContexts.cs

@@ -110,7 +110,7 @@ public override void PopulateMenu(DropdownMenu menu)
                         menu.AppendAction(title, _ => EditorAction.Start(new MenuActionSettings(action, HasPreview(action))), GetStatus(action));
                     }
                     else
-                        menu.AppendAction(GetMenuTitle(action, title), _ => action.PerformAction());
+                        menu.AppendAction(GetMenuTitle(action, title), _ => action.PerformAction(), GetStatus(action));
                 }
             }
 

Samples~/Editor/CustomAction.cs

@@ -35,12 +35,13 @@ public class MakeFacesDoubleSided : MenuAction
 		/// <summary>
 		/// Determines if the action should be enabled or grayed out.
 		/// </summary>
-		/// <returns></returns>
 		public override bool enabled
 		{
-			get { return MeshSelection.selectedFaceCount > 0; }
+			get { return base.enabled && MeshSelection.selectedFaceCount > 0; }
 		}
 
+		protected override bool hasFileMenuEntry => false;
+
 		/// <summary>
 		/// This action is applicable in Face selection modes.
 		/// </summary>
@@ -52,7 +53,6 @@ public override SelectMode validSelectModes
 		/// <summary>
 		/// Return a pb_ActionResult indicating the success/failure of action.
 		/// </summary>
-		/// <returns></returns>
 		protected override ActionResult PerformActionImplementation()
 		{
 			var selection = MeshSelection.top.ToArray();

Tests/Editor/Actions/ContextualMenuTests.cs

@@ -0,0 +1,153 @@
+using NUnit.Framework;
+using UnityEditor;
+using UnityEditor.EditorTools;
+using UnityEditor.ProBuilder;
+using UnityEngine;
+using UnityEngine.ProBuilder;
+using UnityEngine.UIElements;
+
+/// <summary>
+/// Test the construction of the contextual menu in the Scene View.
+/// </summary>
+public class ContextualMenuTests
+{
+    [ProBuilderMenuAction]
+    class ConfigurableMenuAction : MenuAction
+    {
+        internal const string actionName = "Action Without File Menu Entry";
+        internal static bool userHasFileMenuEntry { get; set; }
+        internal static SelectMode userSelectMode { get; set; }
+        internal static bool userEnabled { get; set; }
+
+        public override ToolbarGroup group
+        {
+            get { return ToolbarGroup.Geometry; }
+        }
+
+        public override Texture2D icon => null;
+
+        public override string iconPath => string.Empty;
+
+        public override TooltipContent tooltip => new TooltipContent(
+            actionName,
+            @"This action should not have a file menu entry."
+        );
+
+        public ConfigurableMenuAction()
+        {
+        }
+
+        protected override ActionResult PerformActionImplementation()
+        {
+            return ActionResult.Success;
+        }
+
+        public override SelectMode validSelectModes => userSelectMode;
+        public override bool enabled => userEnabled;
+        protected internal override bool hasFileMenuEntry => userHasFileMenuEntry;
+    }
+
+    ProBuilderMesh m_PBMesh;
+
+    [SetUp]
+    public void Setup()
+    {
+        m_PBMesh = ShapeFactory.Instantiate(typeof(UnityEngine.ProBuilder.Shapes.Plane));
+    }
+
+    [TearDown]
+    public void TearDown()
+    {
+        if (m_PBMesh)
+            Object.DestroyImmediate(m_PBMesh.gameObject);
+    }
+
+    PositionToolContext PrepareToolContext(SelectMode toolSelectMode)
+    {
+        MeshSelection.SetSelection(m_PBMesh.gameObject);
+        ActiveEditorTracker.sharedTracker.ForceRebuild();
+
+        ToolManager.SetActiveContext<PositionToolContext>();
+        Tools.current = Tool.Move;
+        ProBuilderEditor.selectMode = toolSelectMode;
+        ActiveEditorTracker.sharedTracker.ForceRebuild();
+
+        PositionToolContext ctx = Resources.FindObjectsOfTypeAll<PositionToolContext>()?[0];
+        Assume.That(ctx, Is.Not.Null);
+        return ctx;
+    }
+
+    bool CheckDropdownMenuHasItem(string name, DropdownMenuAction.Status status, DropdownMenu menu)
+    {
+        DropdownMenuAction foundInMenu = null;
+        foreach (var t in menu.MenuItems())
+        {
+            if (t is not DropdownMenuAction menuAction)
+                continue;
+
+            if (menuAction.name == name)
+            {
+                foundInMenu = menuAction;
+                break;
+            }
+        }
+
+        return (foundInMenu?.status == status);
+    }
+
+    [Test]
+    [TestCase(true, ExpectedResult = false)]
+    [TestCase(false, ExpectedResult = true)]
+    public bool MenuActionWithoutMenuItem_hasFileMenuEntry(bool hasFileMenuEntry)
+    {
+        var ctx = PrepareToolContext(SelectMode.Face);
+
+        ConfigurableMenuAction.userHasFileMenuEntry = hasFileMenuEntry;
+        ConfigurableMenuAction.userSelectMode = SelectMode.Any;
+        ConfigurableMenuAction.userEnabled = true;
+
+        DropdownMenu menu = new DropdownMenu();
+        ctx.PopulateMenu(menu);
+        menu.PrepareForDisplay(null);
+
+        return CheckDropdownMenuHasItem(ConfigurableMenuAction.actionName, DropdownMenuAction.Status.Normal, menu);
+    }
+
+    [Test]
+    [TestCase(SelectMode.Edge, ExpectedResult = false)]
+    [TestCase(SelectMode.Face, ExpectedResult = true)]
+    [TestCase(SelectMode.Vertex, ExpectedResult = false)]
+    public bool MenuAction_SelectModeSetToFace_EnabledOnlyForFaceSelection(SelectMode mode)
+    {
+        var ctx = PrepareToolContext(mode);
+
+        ConfigurableMenuAction.userHasFileMenuEntry = false;
+        ConfigurableMenuAction.userSelectMode = SelectMode.Face;
+        ConfigurableMenuAction.userEnabled = true;
+
+        DropdownMenu menu = new DropdownMenu();
+        ctx.PopulateMenu(menu);
+        menu.PrepareForDisplay(null);
+
+        // MenuAction is expected to be present in the menu only when the mode matches.
+        return CheckDropdownMenuHasItem(ConfigurableMenuAction.actionName, DropdownMenuAction.Status.Normal, menu);
+    }
+
+    [Test]
+    [TestCase(true, ExpectedResult = true)]
+    [TestCase(false, ExpectedResult = false)]
+    public bool MenuAction_enabledPropertyIsFollowedByMenu(bool enabled)
+    {
+        var ctx = PrepareToolContext(SelectMode.Face);
+
+        ConfigurableMenuAction.userHasFileMenuEntry = false;
+        ConfigurableMenuAction.userSelectMode = SelectMode.Face;
+        ConfigurableMenuAction.userEnabled = enabled;
+
+        DropdownMenu menu = new DropdownMenu();
+        ctx.PopulateMenu(menu);
+        menu.PrepareForDisplay(null);
+
+        return CheckDropdownMenuHasItem(ConfigurableMenuAction.actionName, DropdownMenuAction.Status.Normal, menu);
+    }
+}