Fix event annotation lifecycle and persistence after the sexp_tree refactor

code/missioneditor/sexp_annotation_model.cpp

@@ -55,13 +55,17 @@ void SexpAnnotationModel::saveToGlobal(const SCP_vector<sexp_tree_item>& tree_no
 			int resolved = resolveFromPath(old_path, tree_nodes, events, sig);
 			if (resolved >= 0 || isRootKey(resolved)) {
 				ea.path = buildPath(resolved, tree_nodes, events);
-			} else {
-				// Truly gone; mark default for pruning.
-				ea.comment.clear();
-				ea.r = ea.g = ea.b = 255;
 			}
 		}
 
+		// If no path could be built, the annotated node no longer exists (e.g. its
+		// event was deleted); mark default so prune() removes it rather than letting
+		// an unattachable annotation survive in the global list.
+		if (ea.path.empty()) {
+			ea.comment.clear();
+			ea.r = ea.g = ea.b = 255;
+		}
+
 		// Reset transient field.
 		ea.node_index = -1;
 	}
@@ -77,6 +81,11 @@ void SexpAnnotationModel::saveToGlobal(const SCP_vector<sexp_tree_item>& tree_no
 // Return the vector index of the annotation with the given key, or -1 if not found.
 int SexpAnnotationModel::findByKey(int key) const
 {
+	// -1 is the default/unresolved sentinel, not a real key; matching it would
+	// surface an unresolved annotation on any item that has no key of its own
+	if (key == -1)
+		return -1;
+
 	for (size_t i = 0; i < m_annotations.size(); ++i) {
 		if (m_annotations[i].node_index == key)
 			return static_cast<int>(i);
@@ -202,6 +211,7 @@ SCP_list<int> SexpAnnotationModel::buildPath(int key, const SCP_vector<sexp_tree
 		return path; // root not found in events
 
 	path.push_back(event_idx);
+	path.push_back(0);	// child position under the event label: the top operator is its only child
 
 	// Collect child-position indices from the target node up to the root,
 	// then reverse them so the path reads top-down.
@@ -260,10 +270,16 @@ int SexpAnnotationModel::resolveFromPath(const SCP_list<int>& path, const SCP_ve
 	if (path.size() == 1)
 		return rootKey(formula);
 
-	// Walk down the tree from the formula node.
-	int node = formula;
 	auto it = path.begin();
 	++it; // skip event index
+
+	// A label has exactly one child; any other index fails.
+	if (*it != 0)
+		return -1;
+	++it;
+
+	// Walk down the tree from the formula node.
+	int node = formula;
 	for (; it != path.end() && node >= 0; ++it) {
 		int target = *it;
 		if (node < 0 || node >= static_cast<int>(tree_nodes.size()))

code/missioneditor/sexp_tree_actions.cpp

@@ -1,4 +1,5 @@
 #include "missioneditor/sexp_tree_actions.h"
+#include "missioneditor/sexp_annotation_model.h"
 
 #include "parse/sexp.h"
 #include "parse/sexp_container.h"
@@ -506,6 +507,14 @@ int SexpTreeActions::insert_operator(int op, void* root_parent_handle)
 		if (_model._interface && _model._interface->getFlags()[TreeFlags::LabeledRoot]) {
 			parent_handle = root_parent_handle;
 			_model._interface->onRootInserted(wrapped_node, node);
+
+			// a root label's annotation is keyed to its formula node; re-key it
+			// so it follows the event across the formula change
+			if (_model.annotation_model) {
+				auto* ea = _model.annotation_model->getByKey(SexpAnnotationModel::rootKey(wrapped_node));
+				if (ea)
+					ea->node_index = SexpAnnotationModel::rootKey(node);
+			}
 		} else {
 			_model.root_item = node;
 		}

code/missioneditor/sexp_tree_model.cpp

@@ -396,9 +396,15 @@ void SexpTreeModel::free_node2(int node)
 	total_nodes--;
 
 	// Remove any annotation referencing this node so that if allocate_node()
-	// reuses this slot, the new node won't inherit a stale annotation.
-	if (annotation_model)
+	// reuses this slot, the new node won't inherit a stale annotation.  Also
+	// remove any root-label annotation keyed to this node as an event formula
+	// (rootKey), so that an event whose formula later lands in this slot won't
+	// inherit a deleted event's annotation.  The global Event_annotations are
+	// only rewritten at save, so annotations removed here still survive a Cancel.
+	if (annotation_model) {
 		annotation_model->removeByKey(node);
+		annotation_model->removeByKey(SexpAnnotationModel::rootKey(node));
+	}
 
 	// Recursively free children and following siblings.
 	if (tree_nodes[node].child != -1)

fred2/eventeditor.cpp

@@ -1918,3 +1918,10 @@ SCP_string event_sexp_tree::get_node_comment(int node_index) const
 
 	return "";
 }
+
+SCP_string event_sexp_tree::get_item_comment(HTREEITEM h, int /*node_index*/)
+{
+	// resolves both regular nodes and root labels (which have no model node,
+	// so the base class's node_index would be -1 for them)
+	return get_node_comment(annotation_key_for_item(this, h));
+}

fred2/eventeditor.h

@@ -28,6 +28,7 @@ class event_sexp_tree : public sexp_tree_view
 	void edit_comment(HTREEITEM h);
 	void edit_bg_color(HTREEITEM h);
 	SCP_string get_node_comment(int node_index) const override;
+	SCP_string get_item_comment(HTREEITEM h, int node_index) override;
 
 	// Set by event_editor during initialization
 	SexpAnnotationModel* m_annotations = nullptr;

fred2/sexp_tree_view.cpp

@@ -617,6 +617,13 @@ SCP_string sexp_tree_view::get_node_comment(int /*node_index*/) const
 	return "";
 }
 
+// Returns the comment for a tree item.  The base class only has comments on model
+// nodes; event_sexp_tree overrides this to also resolve labeled event roots.
+SCP_string sexp_tree_view::get_item_comment(HTREEITEM /*h*/, int node_index)
+{
+	return get_node_comment(node_index);
+}
+
 // Handles completion of inline label editing. Validates the new text via
 // _model.validate_label_edit(), commits operator replacements or data changes,
 // and returns 0 if the edit was consumed, 1 if MFC should update the label.
@@ -1759,7 +1766,7 @@ void sexp_tree_view::update_help(HTREEITEM h)
 
 	// Build annotation comment
 	SCP_string nodeComment;
-	SCP_string raw_comment = get_node_comment(node_index);
+	SCP_string raw_comment = get_item_comment(h, node_index);
 	if (!raw_comment.empty()) {
 		nodeComment = "Node Comments:\r\n   " + raw_comment;
 	}

fred2/sexp_tree_view.h

@@ -72,6 +72,7 @@ class sexp_tree_view : public CTreeCtrl, public ISexpTreeUI
 	virtual void edit_comment(HTREEITEM h);
 	virtual void edit_bg_color(HTREEITEM h);
 	virtual SCP_string get_node_comment(int node_index) const;
+	virtual SCP_string get_item_comment(HTREEITEM h, int node_index);
 	void right_clicked();
 	int ctree_size;
 	virtual void build_tree();

qtfred/src/mission/dialogs/MissionEventsDialogModel.cpp

@@ -486,8 +486,10 @@ void MissionEventsDialogModel::reorderByRootFormulaOrder(const SCP_vector<int>&
 	// Keep selection reasonable (select the first event after reorder)
 	setCurrentlySelectedEvent(m_events.empty() ? -1 : 0);
 
-	// Rebuild applied annotations against new node indices/order if needed
-	initializeEventAnnotations();
+	// Annotations are keyed by tree_nodes[] index (or by formula for root labels)
+	// and a root reorder changes neither, so the working annotation set is still
+	// valid.  Do NOT reload it from the global state here (initializeEventAnnotations):
+	// that would discard any annotation edits made since the dialog was opened.
 
 	set_modified();
 }