From 6d13a2556e33ba5bccd1e61061096ba140ff3549 Mon Sep 17 00:00:00 2001 From: Adam Date: Sun, 14 Dec 2025 23:53:22 -0800 Subject: [PATCH] Improve expose input and remove cancel/commit transaction messages --- .../portfolio/document/document_message.rs | 2 - .../document/document_message_handler.rs | 54 ++++++++++--------- .../document/node_graph/node_graph_message.rs | 6 +-- .../node_graph/node_graph_message_handler.rs | 39 +++----------- .../document/node_graph/node_properties.rs | 8 +-- .../utility_types/network_interface.rs | 6 +-- .../tool/tool_messages/freehand_tool.rs | 9 +--- 7 files changed, 45 insertions(+), 79 deletions(-) diff --git a/editor/src/messages/portfolio/document/document_message.rs b/editor/src/messages/portfolio/document/document_message.rs index b6bc9b63ae..cce55e7560 100644 --- a/editor/src/messages/portfolio/document/document_message.rs +++ b/editor/src/messages/portfolio/document/document_message.rs @@ -182,8 +182,6 @@ pub enum DocumentMessage { AddTransaction, StartTransaction, EndTransaction, - CommitTransaction, - CancelTransaction, AbortTransaction, RepeatedAbortTransaction { undo_count: usize, diff --git a/editor/src/messages/portfolio/document/document_message_handler.rs b/editor/src/messages/portfolio/document/document_message_handler.rs index 6402f609fd..6e5b7befdf 100644 --- a/editor/src/messages/portfolio/document/document_message_handler.rs +++ b/editor/src/messages/portfolio/document/document_message_handler.rs @@ -1292,46 +1292,28 @@ impl MessageHandler> for DocumentMes responses.add_front(NodeGraphMessage::RunDocumentGraph); } DocumentMessage::AddTransaction => { - // Reverse order since they are added to the front - responses.add_front(DocumentMessage::CommitTransaction); - responses.add_front(DocumentMessage::StartTransaction); + self.start_transaction(responses); + self.commit_transaction(responses); } // Note: A transaction should never be started in a scope that mutates the network interface, since it will only be run after that scope ends. DocumentMessage::StartTransaction => { - self.network_interface.start_transaction(); - let network_interface_clone = self.network_interface.clone(); - self.document_undo_history.push_back(network_interface_clone); - if self.document_undo_history.len() > crate::consts::MAX_UNDO_HISTORY_LEN { - self.document_undo_history.pop_front(); - } - // Push the UpdateOpenDocumentsList message to the bus in order to update the save status of the open documents - responses.add(PortfolioMessage::UpdateOpenDocumentsList); + self.start_transaction(responses); } // Commits the transaction if the network was mutated since the transaction started, otherwise it cancels the transaction DocumentMessage::EndTransaction => match self.network_interface.transaction_status() { TransactionStatus::Started => { - responses.add_front(DocumentMessage::CancelTransaction); + self.network_interface.finish_transaction(); + self.document_undo_history.pop_back(); } TransactionStatus::Modified => { - responses.add_front(DocumentMessage::CommitTransaction); + self.commit_transaction(responses); } TransactionStatus::Finished => {} }, - DocumentMessage::CancelTransaction => { - self.network_interface.finish_transaction(); - self.document_undo_history.pop_back(); - } - DocumentMessage::CommitTransaction => { - if self.network_interface.transaction_status() == TransactionStatus::Finished { - return; - } - self.network_interface.finish_transaction(); - self.document_redo_history.clear(); - responses.add(PortfolioMessage::UpdateOpenDocumentsList); - } DocumentMessage::AbortTransaction => match self.network_interface.transaction_status() { TransactionStatus::Started => { - responses.add_front(DocumentMessage::CancelTransaction); + self.network_interface.finish_transaction(); + self.document_undo_history.pop_back(); } TransactionStatus::Modified => { responses.add(DocumentMessage::RepeatedAbortTransaction { undo_count: 1 }); @@ -1829,6 +1811,26 @@ impl DocumentMessageHandler { val.unwrap() } + pub fn start_transaction(&mut self, responses: &mut VecDeque) { + self.network_interface.start_transaction(); + let network_interface_clone = self.network_interface.clone(); + self.document_undo_history.push_back(network_interface_clone); + if self.document_undo_history.len() > crate::consts::MAX_UNDO_HISTORY_LEN { + self.document_undo_history.pop_front(); + } + // Push the UpdateOpenDocumentsList message to the bus in order to update the save status of the open documents + responses.add(PortfolioMessage::UpdateOpenDocumentsList); + } + + pub fn commit_transaction(&mut self, responses: &mut VecDeque) { + if self.network_interface.transaction_status() == TransactionStatus::Finished { + return; + } + self.network_interface.finish_transaction(); + self.document_redo_history.clear(); + responses.add(PortfolioMessage::UpdateOpenDocumentsList); + } + pub fn deserialize_document(serialized_content: &str) -> Result { let document_message_handler = serde_json::from_str::(serialized_content) .or_else(|e| { diff --git a/editor/src/messages/portfolio/document/node_graph/node_graph_message.rs b/editor/src/messages/portfolio/document/node_graph/node_graph_message.rs index 330d123efe..c68a68cc7c 100644 --- a/editor/src/messages/portfolio/document/node_graph/node_graph_message.rs +++ b/editor/src/messages/portfolio/document/node_graph/node_graph_message.rs @@ -63,9 +63,9 @@ pub enum NodeGraphMessage { EnterNestedNetwork, DuplicateSelectedNodes, ExposeInput { - input_connector: InputConnector, - set_to_exposed: bool, - start_transaction: bool, + node_id: NodeId, + input_index: usize, + exposed: bool, }, ExposeEncapsulatingPrimaryInput { exposed: bool, diff --git a/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs b/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs index c491b3cba0..88c114b8b5 100644 --- a/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs +++ b/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs @@ -403,15 +403,7 @@ impl<'a> MessageHandler> for NodeG responses.add(DocumentMessage::EnterNestedNetwork { node_id }); } } - NodeGraphMessage::ExposeInput { - input_connector, - set_to_exposed, - start_transaction, - } => { - let InputConnector::Node { node_id, input_index } = input_connector else { - log::error!("Cannot expose/hide export"); - return; - }; + NodeGraphMessage::ExposeInput { node_id, input_index, exposed } => { let Some(node) = network_interface.document_node(&node_id, selection_network_path) else { log::error!("Could not find node {node_id} in NodeGraphMessage::ExposeInput"); return; @@ -421,38 +413,19 @@ impl<'a> MessageHandler> for NodeG return; }; - // If we're un-exposing an input that is not a value, then disconnect it. This will convert it to a value input, - // so we can come back to handle this message again to set the exposed value in the second run-through. - if !set_to_exposed && node_input.as_value().is_none() { - // Reversed order because we are pushing front - responses.add_front(NodeGraphMessage::ExposeInput { - input_connector, - set_to_exposed, - start_transaction: false, - }); - responses.add_front(NodeGraphMessage::DisconnectInput { input_connector }); - responses.add_front(DocumentMessage::StartTransaction); - return; - } + responses.add(DocumentMessage::AddTransaction); - // Add a history step, but only do so if we didn't already start a transaction in the first run-through of this message in the above code - if start_transaction { - responses.add_front(DocumentMessage::StartTransaction); - } - - // If this node's input is a value type, we set its chosen exposed state + let new_exposed = exposed; if let NodeInput::Value { exposed, .. } = &mut node_input { - *exposed = set_to_exposed; + *exposed = new_exposed; } + responses.add(NodeGraphMessage::SetInput { input_connector: InputConnector::node(node_id, input_index), input: node_input, }); - // Finish the history step - responses.add(DocumentMessage::CommitTransaction); - - // Update the graph UI and re-render + // Update the graph UI and re-render if the graph is open, if the graph is closed then open the graph and zoom in on the input if graph_view_overlay_open { responses.add(PropertiesPanelMessage::Refresh); responses.add(NodeGraphMessage::SendGraph); diff --git a/editor/src/messages/portfolio/document/node_graph/node_properties.rs b/editor/src/messages/portfolio/document/node_graph/node_properties.rs index 2ea963345b..c77f218b6c 100644 --- a/editor/src/messages/portfolio/document/node_graph/node_properties.rs +++ b/editor/src/messages/portfolio/document/node_graph/node_properties.rs @@ -47,7 +47,7 @@ pub fn commit_value(_: &T) -> Message { DocumentMessage::AddTransaction.into() } -pub fn expose_widget(node_id: NodeId, index: usize, data_type: FrontendGraphDataType, exposed: bool) -> WidgetInstance { +pub fn expose_widget(node_id: NodeId, input_index: usize, data_type: FrontendGraphDataType, exposed: bool) -> WidgetInstance { ParameterExposeButton::new() .exposed(exposed) .data_type(data_type) @@ -58,9 +58,9 @@ pub fn expose_widget(node_id: NodeId, index: usize, data_type: FrontendGraphData }) .on_update(move |_parameter| Message::Batched { messages: Box::new([NodeGraphMessage::ExposeInput { - input_connector: InputConnector::node(node_id, index), - set_to_exposed: !exposed, - start_transaction: true, + node_id, + input_index, + exposed: !exposed, } .into()]), }) diff --git a/editor/src/messages/portfolio/document/utility_types/network_interface.rs b/editor/src/messages/portfolio/document/utility_types/network_interface.rs index a44c790207..e731e1a872 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface.rs @@ -3800,8 +3800,8 @@ impl NodeNetworkInterface { return; }; - // When changing a NodeInput::Node to a NodeInput::Node, the input should first be disconnected to ensure proper side effects - if (matches!(previous_input, NodeInput::Node { .. }) && matches!(new_input, NodeInput::Node { .. })) { + // When changing a NodeInput::Node to another input, the input should first be disconnected to ensure proper side effects + if matches!(previous_input, NodeInput::Node { .. }) { self.disconnect_input(input_connector, network_path); self.set_input(input_connector, new_input, network_path); return; @@ -3856,7 +3856,7 @@ impl NodeNetworkInterface { return; } - // It is necessary to ensure the grpah is acyclic before calling `self.position` as it sometimes crashes with cyclic graphs #3227 + // It is necessary to ensure the graph is acyclic before calling `self.position` as it sometimes crashes with cyclic graphs #3227 let previous_metadata = match &previous_input { NodeInput::Node { node_id, .. } => self.position(node_id, network_path).map(|position| (*node_id, position)), _ => None, diff --git a/editor/src/messages/tool/tool_messages/freehand_tool.rs b/editor/src/messages/tool/tool_messages/freehand_tool.rs index 9412577b48..88c2511735 100644 --- a/editor/src/messages/tool/tool_messages/freehand_tool.rs +++ b/editor/src/messages/tool/tool_messages/freehand_tool.rs @@ -214,7 +214,6 @@ impl ToolTransition for FreehandTool { #[derive(Clone, Debug, Default)] struct FreehandToolData { end_point: Option<(DVec2, PointId)>, - dragged: bool, weight: f64, layer: Option, } @@ -250,7 +249,6 @@ impl Fsm for FreehandToolFsmState { (FreehandToolFsmState::Ready, FreehandToolMessage::DragStart { append_to_selected }) => { responses.add(DocumentMessage::StartTransaction); - tool_data.dragged = false; tool_data.end_point = None; tool_data.weight = tool_options.line_weight; @@ -307,11 +305,7 @@ impl Fsm for FreehandToolFsmState { FreehandToolFsmState::Drawing } (FreehandToolFsmState::Drawing, FreehandToolMessage::DragStop) => { - if tool_data.dragged { - responses.add(DocumentMessage::CommitTransaction); - } else { - responses.add(DocumentMessage::EndTransaction); - } + responses.add(DocumentMessage::EndTransaction); tool_data.end_point = None; tool_data.layer = None; @@ -380,7 +374,6 @@ fn extend_path_with_next_segment(tool_data: &mut FreehandToolData, position: DVe }); } - tool_data.dragged = true; tool_data.end_point = Some((position, id)); }