Replace NodeTemplate's parallel node and metadata trees with a single flat recursive shape

editor/src/messages/clipboard/clipboard_message_handler.rs

@@ -182,13 +182,8 @@ impl MessageHandler<ClipboardMessage, ClipboardMessageContext<'_>> for Clipboard
 				let mut resource_ids = HashSet::new();
 				for item in &items {
 					match item {
-						ClipboardItem::Layer(entry) => entry
-							.nodes
-							.iter()
-							.for_each(|(_, template)| network_interface::collect_node_resources(&template.document_node, &mut resource_ids)),
-						ClipboardItem::Nodes(nodes) => nodes
-							.iter()
-							.for_each(|(_, template)| network_interface::collect_node_resources(&template.document_node, &mut resource_ids)),
+						ClipboardItem::Layer(entry) => entry.nodes.iter().for_each(|(_, template)| network_interface::collect_template_resources(template, &mut resource_ids)),
+						ClipboardItem::Nodes(nodes) => nodes.iter().for_each(|(_, template)| network_interface::collect_template_resources(template, &mut resource_ids)),
 						ClipboardItem::Vector(_) | ClipboardItem::Resource(_) => {}
 					}
 				}
@@ -256,13 +251,13 @@ impl MessageHandler<ClipboardMessage, ClipboardMessageContext<'_>> for Clipboard
 					match item {
 						ClipboardItem::Layer(mut entry) => {
 							for (_, template) in &mut entry.nodes {
-								template.document_node.normalize_stored_types();
+								template.normalize_stored_types();
 							}
 							layers.push(entry);
 						}
 						ClipboardItem::Nodes(mut nodes) => {
 							for (_, template) in &mut nodes {
-								template.document_node.normalize_stored_types();
+								template.normalize_stored_types();
 							}
 							node_groups.push(nodes);
 						}

editor/src/messages/portfolio/document/node_graph/document_node_definitions/document_node_derive.rs

@@ -1,7 +1,6 @@
 use super::DocumentNodeDefinition;
 use crate::messages::portfolio::document::node_graph::document_node_definitions::DefinitionIdentifier;
-use crate::messages::portfolio::document::utility_types::network_interface::{DocumentNodePersistentMetadata, InputMetadata, NodeTemplate, WidgetOverride};
-use graph_craft::document::*;
+use crate::messages::portfolio::document::utility_types::network_interface::{InputMetadata, NodeTemplate, NodeTemplateImplementation, WidgetOverride};
 use graphene_std::registry::*;
 use graphene_std::*;
 use std::collections::{HashMap, HashSet};
@@ -14,7 +13,7 @@ pub(super) fn post_process_nodes(custom: Vec<DocumentNodeDefinition>) -> HashMap
 	let network_nodes = custom
 		.into_iter()
 		.filter_map(|definition| {
-			if let DocumentNodeImplementation::ProtoNode(proto_node_identifier) = &definition.node_template.document_node.implementation {
+			if let NodeTemplateImplementation::ProtoNode(proto_node_identifier) = &definition.node_template.implementation {
 				definitions_map.insert(DefinitionIdentifier::ProtoNode(proto_node_identifier.clone()), definition);
 				return None;
 			};
@@ -72,29 +71,21 @@ pub(super) fn post_process_nodes(custom: Vec<DocumentNodeDefinition>) -> HashMap
 			DocumentNodeDefinition {
 				identifier: display_name,
 				node_template: NodeTemplate {
-					document_node: DocumentNode {
-						inputs,
-						call_argument: input_type.clone(),
-						implementation: DocumentNodeImplementation::ProtoNode(id.clone()),
-						visible: true,
-						skip_deduplication: false,
-						context_features: ContextDependencies::from(context_features.as_slice()),
-						..Default::default()
-					},
-					persistent_node_metadata: DocumentNodePersistentMetadata {
-						// TODO: Store information for input overrides in the node macro
-						input_metadata: fields
-							.iter()
-							.map(|f| match f.widget_override {
-								RegistryWidgetOverride::None => (f.name, f.description).into(),
-								RegistryWidgetOverride::Hidden => InputMetadata::with_name_description_override(f.name, f.description, WidgetOverride::Hidden),
-								RegistryWidgetOverride::String(str) => InputMetadata::with_name_description_override(f.name, f.description, WidgetOverride::String(str.to_string())),
-								RegistryWidgetOverride::Custom(str) => InputMetadata::with_name_description_override(f.name, f.description, WidgetOverride::Custom(str.to_string())),
-							})
-							.collect(),
-						locked: false,
-						..Default::default()
-					},
+					inputs,
+					call_argument: input_type.clone(),
+					implementation: NodeTemplateImplementation::ProtoNode(id.clone()),
+					context_features: ContextDependencies::from(context_features.as_slice()),
+					// TODO: Store information for input overrides in the node macro
+					input_metadata: fields
+						.iter()
+						.map(|f| match f.widget_override {
+							RegistryWidgetOverride::None => (f.name, f.description).into(),
+							RegistryWidgetOverride::Hidden => InputMetadata::with_name_description_override(f.name, f.description, WidgetOverride::Hidden),
+							RegistryWidgetOverride::String(str) => InputMetadata::with_name_description_override(f.name, f.description, WidgetOverride::String(str.to_string())),
+							RegistryWidgetOverride::Custom(str) => InputMetadata::with_name_description_override(f.name, f.description, WidgetOverride::Custom(str.to_string())),
+						})
+						.collect(),
+					..Default::default()
 				},
 				category,
 				description: Cow::Borrowed(description),
@@ -105,13 +96,14 @@ pub(super) fn post_process_nodes(custom: Vec<DocumentNodeDefinition>) -> HashMap
 
 	// Add the rest of the network nodes to the map and add the metadata for their internal protonodes
 	for mut network_node in network_nodes {
-		traverse_node(&network_node.node_template.document_node, &mut network_node.node_template.persistent_node_metadata, &definitions_map);
+		fill_proto_node_metadata(&mut network_node.node_template, &definitions_map);
+
 		// Set the reference to the node identifier
-		if let Some(nested_metadata) = network_node.node_template.persistent_node_metadata.network_metadata.as_mut() {
-			nested_metadata.persistent_metadata.reference = Some(network_node.identifier.to_string());
+		if let NodeTemplateImplementation::Network(network_template) = &mut network_node.node_template.implementation {
+			network_template.reference = Some(network_node.identifier.to_string());
 			// If it is not a merge node, then set the display name to the identifier/reference
 			if network_node.identifier != "Merge" {
-				network_node.node_template.persistent_node_metadata.display_name = network_node.identifier.to_string();
+				network_node.node_template.display_name = network_node.identifier.to_string();
 			}
 		}
 		definitions_map.insert(DefinitionIdentifier::Network(network_node.identifier.to_string()), network_node);
@@ -120,27 +112,27 @@ pub(super) fn post_process_nodes(custom: Vec<DocumentNodeDefinition>) -> HashMap
 	definitions_map
 }
 
-/// Traverses a document node template and metadata in parallel to add metadata to the protonodes
-fn traverse_node(node: &DocumentNode, node_metadata: &mut DocumentNodePersistentMetadata, definitions_map: &HashMap<DefinitionIdentifier, DocumentNodeDefinition>) {
-	match &node.implementation {
-		DocumentNodeImplementation::Network(node_network) => {
-			for (nested_node_id, nested_node) in node_network.nodes.iter() {
-				let nested_metadata = node_metadata.network_metadata.as_mut().unwrap().persistent_metadata.node_metadata.get_mut(nested_node_id).unwrap();
-				traverse_node(nested_node, &mut nested_metadata.persistent_metadata, definitions_map);
+/// Recursively fills each nested proto node's editor metadata from its definition, preserving only the authored position.
+fn fill_proto_node_metadata(node_template: &mut NodeTemplate, definitions_map: &HashMap<DefinitionIdentifier, DocumentNodeDefinition>) {
+	match &mut node_template.implementation {
+		NodeTemplateImplementation::Network(network_template) => {
+			for nested_template in network_template.nodes.values_mut() {
+				fill_proto_node_metadata(nested_template, definitions_map);
 			}
 		}
-		DocumentNodeImplementation::ProtoNode(id) => {
-			// Set all the metadata except the position to the proto node information from the macro
-			// TODO: Use options in the template to specify what you want to default and what you want to override
-			// If this fails then the proto node id in the definition doesn't match what is generated by the macro
+		NodeTemplateImplementation::ProtoNode(id) => {
+			// If this lookup fails then the proto node id in the definition doesn't match what is generated by the macro
 			let Some(definition) = definitions_map.get(&DefinitionIdentifier::ProtoNode(id.clone())) else {
-				// log::error!("Could not get definition for id {} when filling in protonode metadata for a custom node", id.clone());
 				return;
 			};
-			let mut new_metadata = definition.node_template.persistent_node_metadata.clone();
-			new_metadata.node_type_metadata = node_metadata.node_type_metadata.clone();
-			*node_metadata = new_metadata
+
+			let definition_template = &definition.node_template;
+			node_template.display_name = definition_template.display_name.clone();
+			node_template.input_metadata = definition_template.input_metadata.clone();
+			node_template.output_names = definition_template.output_names.clone();
+			node_template.locked = definition_template.locked;
+			node_template.pinned = definition_template.pinned;
 		}
-		DocumentNodeImplementation::Extract => {}
+		NodeTemplateImplementation::Extract => {}
 	}
 }

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

@@ -301,7 +301,7 @@ impl<'a> MessageHandler<NodeGraphMessage, NodeGraphMessageContext<'a>> for NodeG
 				// A freshly added Text node carries no font, so give it the default font (registered like the Text tool does)
 				if node_type == DefinitionIdentifier::ProtoNode(graphene_std::text::text::IDENTIFIER) {
 					let font_resource_id = graph_craft::application_io::resource::ResourceId::new();
-					if let Some(font_input) = node_template.document_node.inputs.get_mut(graphene_std::text::text::FontInput::INDEX) {
+					if let Some(font_input) = node_template.inputs.get_mut(graphene_std::text::text::FontInput::INDEX) {
 						*font_input = NodeInput::value(TaggedValue::Resource(font_resource_id), false);
 					}
 					responses.add(DocumentMessage::Resource(ResourceMessage::AddFont {
@@ -336,7 +336,7 @@ impl<'a> MessageHandler<NodeGraphMessage, NodeGraphMessageContext<'a>> for NodeG
 					};
 
 					// Ensure connection is to correct input of new node. If it does not have an input then do not connect
-					if let Some((input_index, _)) = node_template.document_node.inputs.iter().enumerate().find(|(_, input)| input.is_exposed()) {
+					if let Some((input_index, _)) = node_template.inputs.iter().enumerate().find(|(_, input)| input.is_exposed()) {
 						responses.add(NodeGraphMessage::CreateWire {
 							output_connector: *output_connector,
 							input_connector: InputConnector::node(node_id, input_index),
@@ -693,7 +693,7 @@ impl<'a> MessageHandler<NodeGraphMessage, NodeGraphMessageContext<'a>> for NodeG
 					return;
 				};
 				let center_of_selected_nodes_grid_space = IVec2::new((center_of_selected_nodes.x / 24. + 0.5).floor() as i32, (center_of_selected_nodes.y / 24. + 0.5).floor() as i32);
-				default_node_template.persistent_node_metadata.node_type_metadata = NodeTypePersistentMetadata::node(center_of_selected_nodes_grid_space - IVec2::new(3, 1));
+				default_node_template.node_type_metadata = NodeTypePersistentMetadata::node(center_of_selected_nodes_grid_space - IVec2::new(3, 1));
 				responses.add(DocumentMessage::AddTransaction);
 				responses.add(NodeGraphMessage::InsertNode {
 					node_id: encapsulating_node_id,

editor/src/messages/portfolio/document/utility_types/network_interface.rs

@@ -10,11 +10,13 @@ mod queries;
 mod resolved_types;
 pub mod storage_metadata;
 mod structure;
+mod template;
 mod types;
 #[cfg(test)]
 mod validation;
 mod view;
 
+pub use template::*;
 pub use types::*;
 pub use view::{NetworkError, NetworkView};
 

editor/src/messages/portfolio/document/utility_types/network_interface/caches.rs

@@ -193,41 +193,21 @@ impl NodeNetworkInterface {
 			let mut not_sole_dependents = HashSet::new();
 			sole_dependents.insert(*stack_top);
 			for upstream_node in self.upstream_flow_back_from_nodes(vec![*stack_top], network_path, FlowType::UpstreamFlow).collect::<Vec<_>>() {
-				let mut stack = vec![upstream_node];
-				let mut is_sole_dependent = true;
-				while let Some(current_node) = stack.pop() {
-					if not_sole_dependents.contains(&current_node) {
-						is_sole_dependent = false;
-						break;
-					}
-					if !sole_dependents.contains(&current_node) {
-						let mut has_outward_wire = false;
-						for output_index in 0..self.number_of_outputs(&current_node, network_path) {
-							let Some(outward_wires) = self.outward_wires(network_path) else {
-								log::error!("Cannot load outward wires in load_stack_dependents");
-								continue;
-							};
-							let Some(outward_wires) = outward_wires.get(&OutputConnector::node(current_node, output_index)) else {
-								log::error!("Cannot load outward wires in load_stack_dependents");
-								continue;
-							};
-							for downstream_input in outward_wires {
-								has_outward_wire = true;
-								match downstream_input {
-									InputConnector::Node { node_id, .. } => stack.push(*node_id),
-									InputConnector::Export(_) => is_sole_dependent = false,
-								}
-							}
-						}
-						if !has_outward_wire {
-							is_sole_dependent = false;
-						}
-					}
-					if !is_sole_dependent {
-						break;
-					}
+				if sole_dependents.contains(&upstream_node) || not_sole_dependents.contains(&upstream_node) {
+					continue;
 				}
 
+				// A path terminates at an already-verified sole dependent, and fails fast through a known non-sole node
+				let is_sole_dependent = self.is_sole_dependent(upstream_node, network_path, |downstream_node, _| {
+					if not_sole_dependents.contains(&downstream_node) {
+						SoleDependentStep::Escape
+					} else if sole_dependents.contains(&downstream_node) {
+						SoleDependentStep::Terminate
+					} else {
+						SoleDependentStep::Continue
+					}
+				});
+
 				if is_sole_dependent {
 					sole_dependents.insert(upstream_node);
 				} else {

editor/src/messages/portfolio/document/utility_types/network_interface/characterization_tests.rs

@@ -73,6 +73,30 @@ async fn deleting_a_node_with_children_prunes_them_from_the_selection() {
 	assert_invariants(&editor, "after deleting a node with children");
 }
 
+#[tokio::test]
+async fn deleting_a_node_keeps_children_shared_with_other_nodes() {
+	let mut editor = EditorTestUtils::create();
+	editor.new_document().await;
+
+	let parent = editor.create_node_by_name(rectangle_definition()).await;
+	let sibling = editor.create_node_by_name(rectangle_definition()).await;
+	let shared_child = editor.create_node_by_name(rectangle_definition()).await;
+
+	let network_interface = &mut editor.active_document_mut().network_interface;
+
+	// Wire the same child into the secondary inputs of both nodes, then delete only the parent along with its children
+	network_interface.set_input(&InputConnector::node(parent, 1), NodeInput::node(shared_child, 0), &[]);
+	network_interface.set_input(&InputConnector::node(sibling, 1), NodeInput::node(shared_child, 0), &[]);
+	network_interface.delete_nodes(vec![parent], true, &[]);
+
+	let nodes = &network_interface.document_network().nodes;
+	assert!(!nodes.contains_key(&parent), "The deleted node itself should be gone");
+	assert!(nodes.contains_key(&shared_child), "A child shared with another node is not a sole dependent and should survive");
+	assert!(nodes.contains_key(&sibling), "The unrelated sibling should survive");
+
+	assert_invariants(&editor, "after deleting a node with a shared child");
+}
+
 #[tokio::test]
 async fn cyclic_connection_is_rejected_without_side_effects() {
 	let mut editor = EditorTestUtils::create();