Keep event annotations attached to their node across tree mutations

Event annotations cache the tree item they point to as a handle (HTREEITEM in
FRED, QTreeWidgetItem* in qtFRED). The handle was silently invalidated whenever
the tree control deleted-and-recreated an item, so annotations got decoupled.

Fix: notify when a node's handle changes. move_branch reports old->new, and
free_node2 / the root-delete path report old->null (deleted). The event tree
remaps the annotation's handle to follow a move, or clears it on delete (the path
is left intact, so a cleared annotation is dropped at save but survives a Cancel).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: Goober5000

fred2/eventeditor.cpp

@@ -1674,6 +1674,21 @@ int event_annotation_lookup(HTREEITEM handle)
 	return -1;
 }
 
+// The tree recreated (moved) or deleted the item for a node.  Re-point any
+// annotation on the old handle to the new one so it follows the node; a null
+// new_handle means the node was deleted, which clears the handle.  The path is
+// left intact, so an annotation cleared this way is dropped at save (its path
+// can no longer be rebuilt) but survives a Cancel (the path still resolves).
+void event_sexp_tree::on_node_handle_changed(HTREEITEM old_handle, HTREEITEM new_handle)
+{
+	if (!old_handle)
+		return;
+
+	int i = event_annotation_lookup(old_handle);
+	if (i >= 0)
+		Event_annotations[i].handle = new_handle;
+}
+
 void event_annotation_swap_image(event_sexp_tree *tree, HTREEITEM handle, int annotation_index)
 {
 	event_annotation_swap_image(tree, handle, Event_annotations[annotation_index]);

fred2/eventeditor.h

@@ -31,6 +31,10 @@ class event_sexp_tree : public sexp_tree
 	virtual void PreSubclassWindow();
 	virtual void OnCustomDraw(NMHDR* pNMHDR, LRESULT* pResult);
 
+	// keep event annotations attached to their node when the tree recreates or
+	// deletes its handle (new_handle == nullptr means the node was deleted)
+	void on_node_handle_changed(HTREEITEM old_handle, HTREEITEM new_handle) override;
+
 	CStringA m_tooltiptextA;
 	CStringW m_tooltiptextW;
 

fred2/sexp_tree.cpp

@@ -479,6 +479,11 @@ void sexp_tree::free_node2(int node)
 	Assert(tree_nodes[node].type != SEXPT_UNUSED);
 	Assert(total_nodes > 0);
 	*modified = 1;
+
+	// the node is being deleted; let subclasses drop anything attached to its handle
+	if (tree_nodes[node].handle)
+		on_node_handle_changed(tree_nodes[node].handle, nullptr);
+
 	tree_nodes[node].type = SEXPT_UNUSED;
 	total_nodes--;
 	if (tree_nodes[node].child != -1)
@@ -2640,6 +2645,9 @@ void sexp_tree::NodeDelete()
 
 		Assert(theNode >= 0);
 		free_node2(theNode);
+		// the event-root item is not a tree_nodes entry, so free_node2 above does
+		// not cover an annotation attached to the event root itself
+		on_node_handle_changed(item_handle, nullptr);
 		DeleteItem(item_handle);
 		*modified = 1;
 		return;
@@ -4668,6 +4676,10 @@ HTREEITEM sexp_tree::move_branch(HTREEITEM source, HTREEITEM parent, HTREEITEM a
   			h = insert(GetItemText(source), image1, image2, parent, after);
 		}
 
+		// the item was recreated with a new handle; let subclasses follow it
+		// (covers both tree_nodes entries and the event-root item, which is not one)
+		on_node_handle_changed(source, h);
+
 		SetItemData(h, GetItemData(source));
 		child = GetChildItem(source);
 		while (child) {

fred2/sexp_tree.h

@@ -367,6 +367,11 @@ class sexp_tree : public CTreeCtrl
 	virtual void NodeReplacePaste();
 	virtual void NodeAddPaste();
 
+	// Notifies that the tree item for a node changed handle (moved/recreated), or
+	// was deleted (new_handle == nullptr).  The event tree overrides this to keep
+	// event annotations attached to their node.  Default: no-op.
+	virtual void on_node_handle_changed(HTREEITEM /*old_handle*/, HTREEITEM /*new_handle*/) {}
+
 	void update_item(HTREEITEM handle);
 
 	int load_branch(int index, int parent);

qtfred/src/mission/dialogs/MissionEventsDialogModel.cpp

@@ -609,8 +609,12 @@ 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 handles/order if needed
-	initializeEventAnnotations();
+	// A root reorder moves top-level items via take/insert, which preserves their
+	// handles (see sexp_tree::move_root), so the cached annotation handles are still
+	// valid and still point at the correct nodes.  Do NOT re-resolve handles from the
+	// stored paths here: the paths carry the pre-reorder event index, so re-resolving
+	// would re-point annotations at the wrong event.  applyAnnotations() rebuilds each
+	// path from its live handle at save time.
 
 	set_modified();
 }
@@ -1138,6 +1142,20 @@ void MissionEventsDialogModel::setNodeBgColor(IEventTreeOps::Handle h, int r, in
 	set_modified();
 }
 
+// The tree recreated (moved) or deleted the item for a node.  Re-point any
+// annotation on the old handle to the new one so it follows the node; a null
+// new_handle means the node was deleted, which clears the cached handle so
+// applyAnnotations() resolves it from the path (and drops it if the node is gone).
+void MissionEventsDialogModel::onNodeHandleChanged(IEventTreeOps::Handle old_handle, IEventTreeOps::Handle new_handle)
+{
+	if (!old_handle)
+		return;
+
+	for (auto& ea : m_event_annotations)
+		if (ea.handle == old_handle)
+			ea.handle = new_handle;
+}
+
 void MissionEventsDialogModel::createMessage()
 {
 	MMessage msg;

qtfred/src/mission/dialogs/MissionEventsDialogModel.h

@@ -142,6 +142,10 @@ class MissionEventsDialogModel : public AbstractDialogModel {
 	void setNodeAnnotation(IEventTreeOps::Handle h, const SCP_string& note);
 	void setNodeBgColor(IEventTreeOps::Handle h, int r, int g, int b, bool has_color);
 
+	// Keep annotations attached to their node when the tree recreates (move) or
+	// deletes (new_handle == nullptr) the item's handle.
+	void onNodeHandleChanged(IEventTreeOps::Handle old_handle, IEventTreeOps::Handle new_handle);
+
 	// Message Management
 	void createMessage();
 	void insertMessage();

qtfred/src/ui/dialogs/MissionEventsDialog.cpp

@@ -245,6 +245,10 @@ void MissionEventsDialog::initEventWidgets() {
 		_model->setNodeBgColor(h, c.red(), c.green(), c.blue(), c.isValid());
 	});
 
+	connect(ui->eventTree, &sexp_tree::nodeHandleChanged, this, [this](void* old_h, void* new_h) {
+		_model->onNodeHandleChanged(old_h, new_h);
+	});
+
 	connect(ui->eventTree, &sexp_tree::rootOrderChanged, this, [this] {
 		SCP_vector<int> order;
 		order.reserve(ui->eventTree->topLevelItemCount());

qtfred/src/ui/widgets/sexp_tree.cpp

@@ -673,6 +673,11 @@ void sexp_tree::free_node2(int node) {
 	Assert(tree_nodes[node].type != SEXPT_UNUSED);
 	Assert(total_nodes > 0);
 	modified();
+
+	// the node is being deleted; let the model drop anything attached to its handle
+	if (tree_nodes[node].handle)
+		nodeHandleChanged(tree_nodes[node].handle, nullptr);
+
 	tree_nodes[node].type = SEXPT_UNUSED;
 	total_nodes--;
 	if (tree_nodes[node].child != -1) {
@@ -2739,6 +2744,9 @@ QTreeWidgetItem* sexp_tree::move_branch(QTreeWidgetItem* source, QTreeWidgetItem
 	h->setData(0, BgColorRole, source->data(0, BgColorRole));
 	applyVisuals(h);
 
+	// the item was recreated with a new handle; let the model follow it
+	nodeHandleChanged(source, h);
+
 	// Move children safely
 	while (source->childCount() > 0) {
 		auto* child = source->child(0);
@@ -7297,6 +7305,10 @@ void sexp_tree::deleteActionHandler()
 			free_node2(formulaNode);
 		}
 
+		// the event-root item is not a tree_nodes entry, so free_node2 above does
+		// not cover an annotation attached to the event root itself
+		nodeHandleChanged(item, nullptr);
+
 		// Remove the UI item and reset selection/index
 		delete item;
 		setCurrentItemIndex(-1);

qtfred/src/ui/widgets/sexp_tree.h

@@ -410,6 +410,11 @@ class sexp_tree: public QTreeWidget {
 	void nodeAnnotationChanged(void* handle, const QString& note);
 	void nodeBgColorChanged(void* handle, const QColor& color);
 
+	// Emitted when a node's tree item changed handle (moved/recreated), or was
+	// deleted (new_handle == nullptr).  Lets the model keep event annotations
+	// attached to their node across tree mutations.
+	void nodeHandleChanged(void* old_handle, void* new_handle);
+
 	// Generated message map functions
  protected:
 	void keyPressEvent(QKeyEvent* e) override;