Keep edits queued behind AddTransaction in their undo step (#4886)

* Make saving an unchanged document write the same bytes, reopened in between or not

* Keep edits queued behind AddTransaction in their undo step

Author: TrueDoctor

editor/src/messages/portfolio/document/document_message_handler.rs

@@ -460,7 +460,10 @@ impl MessageHandler<DocumentMessage, DocumentMessageContext<'_>> for DocumentMes
 				self.layer_range_selection_reference = None;
 				self.chain_node_range_selection_reference = None;
 			}
-			DocumentMessage::DocumentHistoryBackward => self.undo_with_history(document_id, viewport, preferences.validate_storage_round_trip, responses),
+			DocumentMessage::DocumentHistoryBackward => {
+				self.stage_edits_outside_transaction(&resource_storage.resources_mut(), preferences.validate_storage_round_trip);
+				self.undo_with_history(document_id, viewport, preferences.validate_storage_round_trip, responses)
+			}
 			DocumentMessage::DocumentHistoryForward => self.redo_with_history(document_id, viewport, preferences.validate_storage_round_trip, responses),
 			DocumentMessage::DocumentStructureChanged => {
 				if layers_panel_open {
@@ -1655,6 +1658,7 @@ impl MessageHandler<DocumentMessage, DocumentMessageContext<'_>> for DocumentMes
 			}
 			// 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.stage_edits_outside_transaction(&resource_storage.resources_mut(), preferences.validate_storage_round_trip);
 				self.retire_storage_interaction();
 
 				self.network_interface.start_transaction();
@@ -2426,6 +2430,14 @@ impl DocumentMessageHandler {
 		}
 	}
 
+	/// Stages edits made after a transaction finished, such as those queued behind an `AddTransaction`, whose commit runs
+	/// first. They belong to the undo step that just ended, so they must reach storage before it closes.
+	fn stage_edits_outside_transaction(&mut self, byte_store: &dyn graph_craft::application_io::resource::ResourceStorage, validate: bool) {
+		if self.network_interface.transaction_status() == TransactionStatus::Finished && self.network_interface.has_deltas() {
+			self.commit_storage_snapshot(byte_store, validate);
+		}
+	}
+
 	/// The per-peer view settings that ride along with either kind of staging.
 	fn storage_view_settings(&self) -> std::collections::BTreeMap<String, document_graph_storage::Value> {
 		use crate::messages::portfolio::document::utility_types::network_interface::storage_metadata::DocumentSettings;

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

@@ -3235,6 +3235,31 @@ async fn redoing_a_nudge_undone_while_it_glides_leaves_nothing_gliding() {
 	assert!(editor.active_document().network_interface.glide_offsets(&[]).is_empty());
 }
 
+#[tokio::test]
+async fn undoing_a_nudge_keeps_the_node_it_moved() {
+	let mut editor = EditorTestUtils::create();
+	let node = gliding_nudged_node(&mut editor).await;
+
+	editor.handle_message(DocumentMessage::Undo).await;
+	assert_eq!(editor.active_document().network_interface.position(&node, &[]), Some(IVec2::ZERO));
+
+	editor.handle_message(DocumentMessage::Undo).await;
+	assert!(editor.active_document().network_interface.document_network().nodes.is_empty());
+}
+
+#[tokio::test]
+async fn undoing_right_after_creating_a_node_removes_only_that_node() {
+	let mut editor = EditorTestUtils::create();
+	editor.new_document().await;
+	let kept = editor.create_node_by_name_at(rectangle_definition(), 0, 0).await;
+	editor.create_node_by_name_at(rectangle_definition(), 5, 0).await;
+
+	editor.handle_message(DocumentMessage::Undo).await;
+
+	let nodes = &editor.active_document().network_interface.document_network().nodes;
+	assert_eq!(nodes.keys().copied().collect::<Vec<_>>(), vec![kept]);
+}
+
 #[tokio::test]
 async fn entering_a_node_while_its_network_glides_settles_it() {
 	let mut editor = EditorTestUtils::create();

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

@@ -605,6 +605,11 @@ impl NodeNetworkInterface {
 		self.network.network().located_network(location).is_some() && self.network_metadata.nested_metadata(location).is_some()
 	}
 
+	/// Whether writes since the last drain are waiting to be staged.
+	pub(crate) fn has_deltas(&self) -> bool {
+		!self.deltas.is_empty()
+	}
+
 	/// Takes what the writes since the last drain changed, in write order, leaving the buffer empty
 	/// after reading it for the nodes an open transaction has written to.
 	pub(crate) fn take_deltas(&mut self) -> Vec<EditorDelta> {