diff --git a/Coder.Graph/AstGraphEditor.cs b/Coder.Graph/AstGraphEditor.cs index 4109429..ca8ac24 100644 --- a/Coder.Graph/AstGraphEditor.cs +++ b/Coder.Graph/AstGraphEditor.cs @@ -67,6 +67,11 @@ public sealed class AstGraphEditor(AstNode root) /// private const string LinkDropPopup = "ast-graph-link-drop"; + /// + /// The popup offering a node's own actions, opened by right-clicking it. + /// + private const string NodeMenuPopup = "ast-graph-node-menu"; + private string statusMessage = string.Empty; private string fieldBuffer = string.Empty; @@ -94,6 +99,26 @@ public sealed class AstGraphEditor(AstNode root) /// private bool linkDropOpening; + /// + /// The node whose own menu is open, while it is. Null when none is. + /// + /// + /// Held as the AST node rather than the editor's identifier for it, for the reason + /// is: adding a child rebuilds the engine graph and reassigns those + /// identifiers, so a menu remembered by number would go on to grow whichever node inherited it. + /// + private AstNode? nodeMenuNode; + + /// + /// Whether the menu for still has to be opened. + /// + /// + /// Separate from the node for the reason is separate from the pin: + /// only means anything inside a frame, and the request to + /// offer the menu can arrive from outside one. + /// + private bool nodeMenuOpening; + /// /// Gets the graph being edited. /// @@ -221,6 +246,7 @@ public void Draw(Vector2 size, float deltaTime) TrackSelection(); ApplyDeletions(); DrawPalette(); + DrawNodeMenu(); DrawLinkDropMenu(); if (ShowDebugOverlays) @@ -1113,6 +1139,95 @@ public bool RequestCreateFrom(int pinId) return true; } + /// + /// Offers a node's own menu, the way right-clicking the node does. + /// + /// The node to offer the menu for. + /// True if the node is one this graph knows, so the menu will open. + /// + /// Separate from the gesture that usually triggers it, for the reason + /// is: a host binding the menu to a key wants the same menu, and it + /// can be driven in a test without the node happening to be under the pointer. + /// + public bool RequestNodeMenu(int nodeId) + { + AstNode? node = Graph.AstNodeFor(nodeId); + if (node is null) + { + statusMessage = "That node is not in this graph."; + return false; + } + + nodeMenuNode = node; + nodeMenuOpening = true; + return true; + } + + /// + /// Draws a node's own menu, and adds to whichever slot the user picks. + /// + /// + /// What the menu offers is one entry per slot that can hold another child, which is the same edit + /// the inspector's + makes — reachable at the node itself rather than only for the node the + /// inspector has selected. A node with no such slot says so rather than opening empty, the same way + /// the dropped-link menu does. + /// + /// Removing a pin is deliberately not here: the slot is an ordered sequence, so which child a user + /// means is a question the node's own menu cannot ask, and disconnecting the child says it already. + /// + /// + private void DrawNodeMenu() + { + if (nodeMenuNode is not AstNode node) + { + return; + } + + if (nodeMenuOpening) + { + ImGui.OpenPopup(NodeMenuPopup); + nodeMenuOpening = false; + } + + if (!ImGui.BeginPopup(NodeMenuPopup)) + { + // Remembered exactly as long as the popup is open: once it has gone the user either picked + // something or dismissed it, and either way the node is no longer the menu's subject. + nodeMenuNode = null; + return; + } + + IReadOnlyList growable = AstSchema.GrowableSlotsOf(node); + if (growable.Count == 0) + { + ImGui.TextDisabled($"{AstSchema.Describe(node)} has no slot to add to."); + ImGuiProbes.MarkItem("Nothing to add"); + ImGui.EndPopup(); + return; + } + + foreach (AstSlot slot in growable) + { + bool picked = ImGui.MenuItem($"Add {slot.Name}"); + + // Marked under a name of its own rather than the label it reads as, because the inspector + // draws a row for the same edit and a test asking for one should not find the other. + ImGuiProbes.MarkItem($"Node add {slot.Name}"); + + if (!picked) + { + continue; + } + + AddChild(node, slot); + nodeMenuNode = null; + ImGui.CloseCurrentPopup(); + break; + } + + ImGui.EndPopup(); + } + /// /// Creates a node from a template and connects it to the pin a link was dragged off. /// @@ -1408,7 +1523,18 @@ private void DrawPalette() { if (ImGui.IsMouseClicked(ImGuiMouseButton.Right) && ImGui.IsWindowHovered(ImGuiHoveredFlags.RootAndChildWindows)) { - ImGui.OpenPopup("ast-graph-palette"); + // A right-click over a node is about that node, so the palette answers only one over empty + // canvas. Both gestures are the same click, and which menu it means is decided in one place + // rather than by two handlers each opening a popup. + int hovered = 0; + if (ImNodes.IsNodeHovered(ref hovered)) + { + RequestNodeMenu(hovered); + } + else + { + ImGui.OpenPopup("ast-graph-palette"); + } } if (!ImGui.BeginPopup("ast-graph-palette")) diff --git a/Coder.Graph/AstSchema.cs b/Coder.Graph/AstSchema.cs index 8fd5f7d..67c9856 100644 --- a/Coder.Graph/AstSchema.cs +++ b/Coder.Graph/AstSchema.cs @@ -107,6 +107,20 @@ public static class AstSchema _ => [], }; + /// + /// Lists the slots of a node that a child can be added to without displacing one already there. + /// + /// The node to describe. + /// The node's variadic slots, in the order the editor draws their pins. Empty when it has none. + /// + /// A slot holding at most one child is left out rather than offered and refused: connecting a + /// second child to one replaces the first, so adding to it is not a thing the node can be asked + /// for. Which slots these are is a fact about the node's shape, so it is answered here rather than + /// by whatever draws the menu. + /// + public static IReadOnlyList GrowableSlotsOf(AstNode node) => + [.. SlotsOf(node).Where(slot => slot.Cardinality == AstSlotCardinality.Many)]; + /// /// Reads the children currently sitting in a slot. /// diff --git a/Coder.Test/Graph/AstGraphEditorNodeMenuTests.cs b/Coder.Test/Graph/AstGraphEditorNodeMenuTests.cs new file mode 100644 index 0000000..e027127 --- /dev/null +++ b/Coder.Test/Graph/AstGraphEditorNodeMenuTests.cs @@ -0,0 +1,240 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Coder.Test.Graph; + +using System.Linq; +using System.Numerics; +using Hexa.NET.ImGui; +using Hexa.NET.ImNodes; +using ktsu.Coder.Ast; +using ktsu.Coder.Graph; +using ktsu.ImGui.App; +using ktsu.ImGui.App.Testing; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Covers the menu a right-clicked node opens as the user meets it: entries reached with the mouse, +/// by the label they read as, through the headless rasterizer. +/// +/// +/// Which slots the menu offers is a fact about the node's shape and is covered headlessly in +/// . What is covered here is that the popup draws an entry per slot at +/// all, that picking one reaches the edit, and that a right-click over a node opens this menu rather +/// than the create-node palette — the half that only a rendered frame can establish, since ImGui +/// menus fault inside native code rather than throwing when misnested. +/// +/// The menu is opened through in every test but the +/// gesture's own, because where a node lands on screen depends on the force-directed layout. The one +/// test that does click the node stops the layout first and asks ImNodes where the node ended up. +/// +/// +/// ImGui contexts are process-global, so only one harness can be live at a time and this class must +/// not run its methods in parallel. +/// +/// +[TestClass] +[DoNotParallelize] +public sealed class AstGraphEditorNodeMenuTests +{ + private static readonly HarnessOptions Options = new() { Width = 1280, Height = 800 }; + + private static FunctionDeclaration SampleFunction() + { + FunctionDeclaration function = new("total") { ReturnType = "int" }; + function.Parameters.Add(new Parameter("a", "int")); + function.Body.Add(new ReturnStatement( + new BinaryExpression(new VariableReference("a"), BinaryOperator.Add, Literal.Number(1)))); + return function; + } + + private static ImGuiAppConfig ConfigFor(AstGraphEditor editor) => new() + { + Title = "AST node menu", + OnRender = delta => + { + ImGui.Begin("graph"); + editor.Draw(new Vector2(1000, 600), delta); + ImGui.End(); + }, + }; + + private static bool IsVisible(ImGuiAppHarness harness, string name) => + harness.Probe.WasSeenInFrame(name, harness.FrameCount - 1); + + /// + /// Finds the editor node standing for an AST node, the way the editor does. + /// + /// The graph under test. + /// The AST node to find. + /// The editor node's id. + private static int NodeId(AstGraph graph, AstNode node) => + graph.Engine.Nodes.Single(n => ReferenceEquals(graph.AstNodeFor(n.Id), node)).Id; + + /// + /// Starts a harness with a node's own menu already open. + /// + /// The editor to draw. + /// The node to open the menu for. + /// The running harness, for the caller to dispose. + private static ImGuiAppHarness Offering(AstGraphEditor editor, AstNode node) + { + ImGuiAppHarness harness = ImGuiAppHarness.Start(ConfigFor(editor), Options); + harness.Step(2); + + Assert.IsTrue(editor.RequestNodeMenu(NodeId(editor.Graph, node)), "the node to offer a menu for should be in the graph"); + harness.Step(2); + return harness; + } + + /// + /// Tests that the menu offers one entry per slot the node can hold another child in. + /// + [TestMethod] + public void Menu_OffersAnEntryPerSlotThatCanGrow() + { + FunctionDeclaration function = SampleFunction(); + AstGraphEditor editor = new(function); + + using ImGuiAppHarness harness = Offering(editor, function); + + Assert.IsTrue(IsVisible(harness, "Node add Parameters"), "a function's parameter list can take another"); + Assert.IsTrue(IsVisible(harness, "Node add Body"), "a function's body can take another statement"); + Assert.IsFalse(IsVisible(harness, "Nothing to add"), "a function has slots to add to, so the menu is not empty"); + } + + /// + /// Tests that picking an entry adds a child to that slot, through the menu rather than by calling + /// the edit directly, and that the step is undoable. + /// + [TestMethod] + public void Menu_AddsToThePickedSlot() + { + FunctionDeclaration function = SampleFunction(); + AstGraphEditor editor = new(function); + + using ImGuiAppHarness harness = Offering(editor, function); + + harness.Click("Node add Parameters"); + harness.Step(2); + + Assert.HasCount(2, function.Parameters); + Assert.HasCount(1, function.Body); + Assert.IsTrue(editor.History.CanUndo, "adding through the menu should be undoable"); + + editor.Undo(); + Assert.HasCount(1, function.Parameters); + } + + /// + /// Tests that a node whose every slot holds one child says so, rather than opening an empty menu. + /// + [TestMethod] + public void Menu_SaysSoWhenTheNodeHasNoSlotToAddTo() + { + BinaryExpression sum = new(new VariableReference("a"), BinaryOperator.Add, Literal.Number(1)); + AstGraphEditor editor = new(sum); + + using ImGuiAppHarness harness = Offering(editor, sum); + + Assert.IsTrue(IsVisible(harness, "Nothing to add"), "both of a binary expression's slots hold one child"); + Assert.IsFalse(IsVisible(harness, "Node add Left"), "a slot holding one child is not offered"); + } + + /// + /// Tests that dismissing the menu without picking anything leaves the document alone and forgets the + /// node, rather than offering the menu again on the next frame. + /// + [TestMethod] + public void Menu_DismissedWithoutPicking_ChangesNothing() + { + FunctionDeclaration function = SampleFunction(); + AstGraphEditor editor = new(function); + + using ImGuiAppHarness harness = Offering(editor, function); + Assert.IsTrue(IsVisible(harness, "Node add Parameters"), "the menu should be open to begin with"); + + int before = editor.Graph.Nodes.Count; + + // Clicked away from the popup rather than dismissed with Escape: a click outside is what closes + // an ImGui popup, and it is also what a user who changed their mind actually does. + harness.Mouse.Click(1220, 60); + harness.Step(3); + + Assert.IsFalse(IsVisible(harness, "Node add Parameters"), "the menu should be gone once dismissed"); + Assert.HasCount(before, editor.Graph.Nodes); + Assert.IsFalse(editor.History.CanUndo, "dismissing should record nothing"); + Assert.HasCount(1, function.Parameters); + } + + /// + /// Tests that right-clicking a node opens that node's menu, where right-clicking empty canvas still + /// opens the create-node palette. + /// + /// + /// The layout is stopped and the view left where the first frames fitted it, so the node is where + /// ImNodes last put it rather than somewhere the simulation has since moved it. + /// + [TestMethod] + public void RightClickOnANode_OpensThatNodesMenu() + { + FunctionDeclaration function = SampleFunction(); + AstGraphEditor editor = new(function) { LayoutRunning = false }; + + Vector2 topLeft = Vector2.Zero; + Vector2 size = Vector2.Zero; + int nodeId = 0; + + ImGuiAppConfig config = new() + { + Title = "AST node menu gesture", + OnRender = delta => + { + ImGui.Begin("graph"); + editor.Draw(new Vector2(1000, 600), delta); + + // Read inside the frame that drew it: the node's screen position is ImNodes' own, and it + // is only meaningful while its context is the current one. + if (nodeId != 0) + { + topLeft = ImNodes.GetNodeScreenSpacePos(nodeId); + size = ImNodes.GetNodeDimensions(nodeId); + } + + ImGui.End(); + }, + }; + + using ImGuiAppHarness harness = ImGuiAppHarness.Start(config, Options); + harness.Step(4); + + nodeId = NodeId(editor.Graph, function); + harness.Step(2); + + Assert.IsGreaterThan(0f, size.X, "the node should have been measured by now"); + + // The title bar rather than the middle of the body: a node's rows are the pins, and a click + // landing on one is a different gesture. + harness.Mouse.Click(topLeft.X + (size.X * 0.5f), topLeft.Y + 4f, 1); + harness.Step(2); + + Assert.IsTrue(IsVisible(harness, "Node add Parameters"), "right-clicking the node should open its own menu"); + } + + /// + /// Tests that a node that is not in the graph is refused, so no menu is offered for it. + /// + [TestMethod] + public void RequestNodeMenu_UnknownNode_IsRefused() + { + AstGraphEditor editor = new(SampleFunction()); + + using ImGuiAppHarness harness = ImGuiAppHarness.Start(ConfigFor(editor), Options); + harness.Step(2); + + Assert.IsFalse(editor.RequestNodeMenu(-1)); + harness.Step(2); + + Assert.IsFalse(IsVisible(harness, "Node add Parameters")); + Assert.IsFalse(IsVisible(harness, "Nothing to add")); + } +} diff --git a/Coder.Test/Graph/AstSchemaTests.cs b/Coder.Test/Graph/AstSchemaTests.cs index 0ebd115..a9a82e5 100644 --- a/Coder.Test/Graph/AstSchemaTests.cs +++ b/Coder.Test/Graph/AstSchemaTests.cs @@ -424,6 +424,34 @@ public void Describe_FallsBackToTheNodeTypeName() => public void Describe_HandlesANullLiteralValue() => Assert.AreEqual("\"\"", AstSchema.Describe(new LiteralExpression())); + /// + /// Tests that the slots a child can be added to are the variadic ones, in the order the editor + /// draws their pins. + /// + [TestMethod] + public void GrowableSlotsOf_KeepsOnlyTheSlotsThatHoldASequence() => + Assert.AreSequenceEqual( + FunctionSlots, + AstSchema.GrowableSlotsOf(new FunctionDeclaration("f")).Select(s => s.Name)); + + /// + /// Tests that a node whose every slot holds one child has none to add to, since connecting a second + /// child to such a slot replaces the first rather than joining it. + /// + [TestMethod] + public void GrowableSlotsOf_IsEmptyWhenEverySlotHoldsOneChild() + { + Assert.IsEmpty(AstSchema.GrowableSlotsOf(NewBinary())); + Assert.IsEmpty(AstSchema.GrowableSlotsOf(new ReturnStatement())); + } + + /// + /// Tests that a leaf, which has no slots at all, has none to add to either. + /// + [TestMethod] + public void GrowableSlotsOf_IsEmptyForALeaf() => + Assert.IsEmpty(AstSchema.GrowableSlotsOf(new VariableReference("a"))); + private static BinaryExpression NewBinary() => new(new VariableReference("a"), BinaryOperator.Add, new VariableReference("b"));