Partly fix when the "Make Path Editable" button is shown as enabled (#2968)

* Fix add path node button enable

* Fix add path node button enable

* Refactor code

* Fix formatting

* Clean up logic

---------

Co-authored-by: Keavon Chambers <keavon@keavon.com>

editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs

@@ -17,11 +17,11 @@ use crate::messages::portfolio::document::utility_types::wires::{GraphWireStyle,
 use crate::messages::prelude::*;
 use crate::messages::tool::common_functionality::auto_panning::AutoPanning;
 use crate::messages::tool::common_functionality::graph_modification_utils::{self, get_clip_mode};
+use crate::messages::tool::common_functionality::utility_functions::make_path_editable_is_allowed;
 use crate::messages::tool::tool_messages::tool_prelude::{Key, MouseMotion};
 use crate::messages::tool::utility_types::{HintData, HintGroup, HintInfo};
 use bezier_rs::Subpath;
 use glam::{DAffine2, DVec2, IVec2};
-use graph_craft::document::value::TaggedValue;
 use graph_craft::document::{DocumentNodeImplementation, NodeId, NodeInput};
 use graph_craft::proto::GraphErrors;
 use graphene_std::math::math_ext::QuadExt;
@@ -126,35 +126,9 @@ impl<'a> MessageHandler<NodeGraphMessage, NodeGraphMessageContext<'a>> for NodeG
 				responses.add(NodeGraphMessage::SelectedNodesSet { nodes: vec![new_layer_id] });
 			}
 			NodeGraphMessage::AddPathNode => {
-				let selected_nodes = network_interface.selected_nodes();
+				if let Some(layer) = make_path_editable_is_allowed(network_interface, network_interface.document_metadata()) {
-				let mut selected_layers = selected_nodes.selected_layers(network_interface.document_metadata());
+					responses.add(NodeGraphMessage::CreateNodeInLayerWithTransaction { node_type: "Path".to_string(), layer });
-				let first_layer = selected_layers.next();
+					responses.add(BroadcastEvent::SelectionChanged);
-				let second_layer = selected_layers.next();
-				let has_single_selection = first_layer.is_some() && second_layer.is_none();
-
-				let compatible_type = first_layer.and_then(|layer| {
-					let graph_layer = graph_modification_utils::NodeGraphLayer::new(layer, &network_interface);
-					graph_layer.horizontal_layer_flow().nth(1).and_then(|node_id| {
-						let (output_type, _) = network_interface.output_type(&node_id, 0, &[]);
-						Some(format!("type:{}", output_type.nested_type()))
-					})
-				});
-
-				let is_compatible = compatible_type.as_deref() == Some("type:Instances<VectorData>");
-
-				if first_layer.is_some() && has_single_selection && is_compatible {
-					if let Some(layer) = first_layer {
-						let node_type = "Path".to_string();
-						let graph_layer = graph_modification_utils::NodeGraphLayer::new(layer, &network_interface);
-						let is_modifiable = matches!(graph_layer.find_input("Path", 1), Some(TaggedValue::VectorModification(_)));
-						if !is_modifiable {
-							responses.add(NodeGraphMessage::CreateNodeInLayerWithTransaction {
-								node_type: node_type.clone(),
-								layer: LayerNodeIdentifier::new_unchecked(layer.to_node()),
-							});
-							responses.add(BroadcastEvent::SelectionChanged);
-						}
-					}
 				}
 			}
 			NodeGraphMessage::AddImport => {

editor/src/messages/portfolio/menu_bar/menu_bar_message_handler.rs

@@ -19,7 +19,7 @@ pub struct MenuBarMessageHandler {
 	pub spreadsheet_view_open: bool,
 	pub message_logging_verbosity: MessageLoggingVerbosity,
 	pub reset_node_definitions_on_open: bool,
-	pub single_path_node_compatible_layer_selected: bool,
+	pub make_path_editable_is_allowed: bool,
 }
 
 #[message_handler_data]
@@ -46,7 +46,7 @@ impl LayoutHolder for MenuBarMessageHandler {
 		let message_logging_verbosity_names = self.message_logging_verbosity == MessageLoggingVerbosity::Names;
 		let message_logging_verbosity_contents = self.message_logging_verbosity == MessageLoggingVerbosity::Contents;
 		let reset_node_definitions_on_open = self.reset_node_definitions_on_open;
-		let single_path_node_compatible_layer_selected = self.single_path_node_compatible_layer_selected;
+		let make_path_editable_is_allowed = self.make_path_editable_is_allowed;
 
 		let menu_bar_entries = vec![
 			MenuBarEntry {
@@ -442,7 +442,7 @@ impl LayoutHolder for MenuBarMessageHandler {
 						icon: Some("NodeShape".into()),
 						shortcut: None,
 						action: MenuBarEntry::create_action(|_| NodeGraphMessage::AddPathNode.into()),
-						disabled: !single_path_node_compatible_layer_selected,
+						disabled: !make_path_editable_is_allowed,
 						..MenuBarEntry::default()
 					}],
 				]),

editor/src/messages/portfolio/portfolio_message_handler.rs

@@ -20,12 +20,12 @@ use crate::messages::portfolio::document_migration::*;
 use crate::messages::preferences::SelectionMode;
 use crate::messages::prelude::*;
 use crate::messages::tool::common_functionality::graph_modification_utils;
+use crate::messages::tool::common_functionality::utility_functions::make_path_editable_is_allowed;
 use crate::messages::tool::utility_types::{HintData, HintGroup, ToolType};
 use crate::node_graph_executor::{ExportConfig, NodeGraphExecutor};
 use bezier_rs::BezierHandles;
 use glam::{DAffine2, DVec2};
 use graph_craft::document::NodeId;
-use graph_craft::document::value::TaggedValue;
 use graphene_std::Color;
 use graphene_std::renderer::Quad;
 use graphene_std::text::Font;
@@ -84,7 +84,7 @@ impl MessageHandler<PortfolioMessage, PortfolioMessageContext<'_>> for Portfolio
 				self.menu_bar_message_handler.has_selected_nodes = false;
 				self.menu_bar_message_handler.has_selected_layers = false;
 				self.menu_bar_message_handler.has_selection_history = (false, false);
-				self.menu_bar_message_handler.single_path_node_compatible_layer_selected = false;
+				self.menu_bar_message_handler.make_path_editable_is_allowed = false;
 				self.menu_bar_message_handler.spreadsheet_view_open = self.spreadsheet.spreadsheet_view_open;
 				self.menu_bar_message_handler.message_logging_verbosity = message_logging_verbosity;
 				self.menu_bar_message_handler.reset_node_definitions_on_open = reset_node_definitions_on_open;
@@ -102,30 +102,7 @@ impl MessageHandler<PortfolioMessage, PortfolioMessageContext<'_>> for Portfolio
 						let metadata = &document.network_interface.document_network_metadata().persistent_metadata;
 						(!metadata.selection_undo_history.is_empty(), !metadata.selection_redo_history.is_empty())
 					};
-					self.menu_bar_message_handler.single_path_node_compatible_layer_selected = {
+					self.menu_bar_message_handler.make_path_editable_is_allowed = make_path_editable_is_allowed(&document.network_interface, document.metadata()).is_some();
-						let selected_nodes = document.network_interface.selected_nodes();
-						let mut selected_layers = selected_nodes.selected_layers(document.metadata());
-						let first_layer = selected_layers.next();
-						let second_layer = selected_layers.next();
-						let has_single_selection = first_layer.is_some() && second_layer.is_none();
-
-						let compatible_type = first_layer.and_then(|layer| {
-							let graph_layer = graph_modification_utils::NodeGraphLayer::new(layer, &document.network_interface);
-							graph_layer.horizontal_layer_flow().nth(1).map(|node_id| {
-								let (output_type, _) = document.network_interface.output_type(&node_id, 0, &[]);
-								format!("type:{}", output_type.nested_type())
-							})
-						});
-
-						let is_compatible = compatible_type.as_deref() == Some("type:Instances<VectorData>");
-
-						let is_modifiable = first_layer.is_some_and(|layer| {
-							let graph_layer = graph_modification_utils::NodeGraphLayer::new(layer, &document.network_interface);
-							matches!(graph_layer.find_input("Path", 1), Some(TaggedValue::VectorModification(_)))
-						});
-
-						first_layer.is_some() && has_single_selection && is_compatible && !is_modifiable
-					}
 				}
 
 				self.menu_bar_message_handler.process_message(message, responses, ());

editor/src/messages/tool/common_functionality/utility_functions.rs

@@ -1,18 +1,21 @@
 use super::snapping::{SnapCandidatePoint, SnapData, SnapManager};
 use super::transformation_cage::{BoundingBoxManager, SizeSnapData};
 use crate::consts::ROTATE_INCREMENT;
-use crate::messages::portfolio::document::utility_types::document_metadata::LayerNodeIdentifier;
+use crate::messages::portfolio::document::utility_types::document_metadata::{DocumentMetadata, LayerNodeIdentifier};
+use crate::messages::portfolio::document::utility_types::network_interface::NodeNetworkInterface;
 use crate::messages::portfolio::document::utility_types::transformation::Selected;
 use crate::messages::prelude::*;
-use crate::messages::tool::common_functionality::graph_modification_utils::get_text;
+use crate::messages::tool::common_functionality::graph_modification_utils::{NodeGraphLayer, get_text};
 use crate::messages::tool::common_functionality::transformation_cage::SelectedEdges;
 use crate::messages::tool::tool_messages::path_tool::PathOverlayMode;
 use crate::messages::tool::utility_types::ToolType;
 use bezier_rs::{Bezier, BezierHandles};
 use glam::{DAffine2, DVec2};
+use graph_craft::concrete;
+use graph_craft::document::value::TaggedValue;
 use graphene_std::renderer::Quad;
 use graphene_std::text::{FontCache, load_font};
-use graphene_std::vector::{HandleExt, HandleId, ManipulatorPointId, PointId, SegmentId, VectorData, VectorModificationType};
+use graphene_std::vector::{HandleExt, HandleId, ManipulatorPointId, PointId, SegmentId, VectorData, VectorDataTable, VectorModification, VectorModificationType};
 use kurbo::{CubicBez, Line, ParamCurveExtrema, PathSeg, Point, QuadBez};
 
 /// Determines if a path should be extended. Goal in viewport space. Returns the path and if it is extending from the start, if applicable.
@@ -586,3 +589,36 @@ pub fn find_two_param_best_approximate(p1: DVec2, p3: DVec2, d1: DVec2, d2: DVec
 
 	(d1 * len1, d2 * len2)
 }
+
+pub fn make_path_editable_is_allowed(network_interface: &NodeNetworkInterface, metadata: &DocumentMetadata) -> Option<LayerNodeIdentifier> {
+	// Must have exactly one layer selected
+	let selected_nodes = network_interface.selected_nodes();
+	let mut selected_layers = selected_nodes.selected_layers(metadata);
+	let first_layer = selected_layers.next()?;
+	if selected_layers.next().is_some() {
+		return None;
+	}
+
+	// Must be a layer of type VectorDataTable
+	let compatible_type = NodeGraphLayer::new(first_layer, network_interface)
+		.horizontal_layer_flow()
+		.nth(1)
+		.map(|node_id| {
+			let (output_type, _) = network_interface.output_type(&node_id, 0, &[]);
+			output_type.nested_type() == concrete!(VectorDataTable).nested_type()
+		})
+		.unwrap_or_default();
+	if !compatible_type {
+		return None;
+	}
+
+	// Must not already have an existing Path node, in the right-most part of the layer chain, which has an empty set of modifications
+	// (otherwise users could repeatedly keep running this command and stacking up empty Path nodes)
+	if let Some(TaggedValue::VectorModification(modifications)) = NodeGraphLayer::new(first_layer, network_interface).find_input("Path", 1) {
+		if modifications.as_ref() == &VectorModification::default() {
+			return None;
+		}
+	}
+
+	Some(first_layer)
+}

editor/src/messages/tool/tool_messages/path_tool.rs

@@ -20,9 +20,8 @@ use crate::messages::tool::common_functionality::shape_editor::{
 	ClosestSegment, ManipulatorAngle, OpposingHandleLengths, SelectedLayerState, SelectedPointsInfo, SelectionChange, SelectionShape, SelectionShapeType, ShapeState,
 };
 use crate::messages::tool::common_functionality::snapping::{SnapCache, SnapCandidatePoint, SnapConstraint, SnapData, SnapManager};
-use crate::messages::tool::common_functionality::utility_functions::{calculate_segment_angle, find_two_param_best_approximate};
+use crate::messages::tool::common_functionality::utility_functions::{calculate_segment_angle, find_two_param_best_approximate, make_path_editable_is_allowed};
 use bezier_rs::{Bezier, BezierHandles, TValue};
-use graph_craft::document::value::TaggedValue;
 use graphene_std::Color;
 use graphene_std::renderer::Quad;
 use graphene_std::transform::ReferencePoint;
@@ -291,7 +290,7 @@ impl LayoutHolder for PathTool {
 			.icon(Some("NodeShape".into()))
 			.tooltip("Make Path Editable")
 			.on_update(|_| NodeGraphMessage::AddPathNode.into())
-			.disabled(!self.tool_data.single_path_node_compatible_layer_selected)
+			.disabled(!self.tool_data.make_path_editable_is_allowed)
 			.widget_holder();
 
 		let [_checkbox, _dropdown] = {
@@ -571,7 +570,7 @@ struct PathToolData {
 	drill_through_cycle_count: usize,
 	hovered_layers: Vec<LayerNodeIdentifier>,
 	ghost_outline: Vec<(Vec<ClickTargetType>, LayerNodeIdentifier)>,
-	single_path_node_compatible_layer_selected: bool,
+	make_path_editable_is_allowed: bool,
 }
 
 impl PathToolData {
@@ -3014,30 +3013,7 @@ impl Fsm for PathToolFsmState {
 					colinear,
 				};
 
-				tool_data.single_path_node_compatible_layer_selected = {
+				tool_data.make_path_editable_is_allowed = make_path_editable_is_allowed(&document.network_interface, document.metadata()).is_some();
-					let selected_nodes = document.network_interface.selected_nodes();
-					let mut selected_layers = selected_nodes.selected_layers(document.metadata());
-					let first_layer = selected_layers.next();
-					let second_layer = selected_layers.next();
-					let has_single_selection = first_layer.is_some() && second_layer.is_none();
-
-					let compatible_type = first_layer.and_then(|layer| {
-						let graph_layer = graph_modification_utils::NodeGraphLayer::new(layer, &document.network_interface);
-						graph_layer.horizontal_layer_flow().nth(1).map(|node_id| {
-							let (output_type, _) = document.network_interface.output_type(&node_id, 0, &[]);
-							format!("type:{}", output_type.nested_type())
-						})
-					});
-
-					let is_compatible = compatible_type.as_deref() == Some("type:Instances<VectorData>");
-
-					let is_modifiable = first_layer.is_some_and(|layer| {
-						let graph_layer = graph_modification_utils::NodeGraphLayer::new(layer, &document.network_interface);
-						matches!(graph_layer.find_input("Path", 1), Some(TaggedValue::VectorModification(_)))
-					});
-
-					first_layer.is_some() && has_single_selection && is_compatible && !is_modifiable
-				};
 				tool_data.update_selection_status(shape_editor, document);
 				self
 			}