fix a few campaign editor bugs

1. Properly initialize campaign links and elements so that stale values don't stick around from previously loaded campaigns.
2. Clear internal flag bits from campaign missions to prevent invalid state from being loaded from a campaign file.
3. Properly check the main hall when saving flags in retail format.
4. Clean up parsing and saving of mission loop data, which had been broken ever since retail.  Add compatibility checks for the old format.

code/mission/missioncampaign.cpp

@@ -550,6 +550,9 @@ int mission_campaign_load(const char* filename, const char* full_path, player* p
 				cm->main_hall = "1";
 			}
 
+			// clear any other flag bits to prevent bogus values causing surprises
+			cm->flags &= CMISSION_EXTERNAL_FLAG_MASK;
+
 			// Goober5000 - new main hall stuff!
 			// Updated by CommanderDJ
 			if (optional_string("+Main Hall:")) {
@@ -610,12 +613,16 @@ int mission_campaign_load(const char* filename, const char* full_path, player* p
 
 			cm->mission_branch_brief_anim = NULL;
 			if ( optional_string("+Mission Loop Brief Anim:") || optional_string("+Mission Fork Brief Anim:") ) {
-				cm->mission_branch_brief_anim = stuff_and_malloc_string(F_MULTITEXT, NULL);
+				ignore_white_space();						// it might be on the next line
+				cm->mission_branch_brief_anim = stuff_and_malloc_string(F_FILESPEC, nullptr);
+				(void)optional_string("$end_multi_text");	// consume the unneeded ending token
 			}
 
 			cm->mission_branch_brief_sound = NULL;
 			if ( optional_string("+Mission Loop Brief Sound:") || optional_string("+Mission Fork Brief Sound:") ) {
-				cm->mission_branch_brief_sound = stuff_and_malloc_string(F_MULTITEXT, NULL);
+				ignore_white_space();						// it might be on the next line
+				cm->mission_branch_brief_sound = stuff_and_malloc_string(F_FILESPEC, nullptr);
+				(void)optional_string("$end_multi_text");	// consume the unneeded ending token
 			}
 
 			cm->mission_loop_formula = -1;

code/missioneditor/campaignsave.cpp

@@ -134,7 +134,7 @@ int Fred_campaign_save::save_campaign_file(const char* pathname, const SCP_vecto
 			fout(" %s", cm.main_hall.c_str());
 		} else {
 			// save Bastion flag properly
-			fout(" %d", flags_to_save | ((!cm.main_hall.empty()) ? CMISSION_FLAG_BASTION : 0));
+			fout(" %d", flags_to_save | ((!cm.main_hall.empty() && cm.main_hall != "0") ? CMISSION_FLAG_BASTION : 0));
 		}
 
 		if (!cm.substitute_main_hall.empty()) {
@@ -218,23 +218,35 @@ int Fred_campaign_save::save_campaign_file(const char* pathname, const SCP_vecto
 					}
 
 					if ((num_mission_special == 1) && link.mission_branch_brief_anim) {
-						if (mission_loop)
-							required_string_fred("+Mission Loop Brief Anim:");
+						auto str = mission_loop ? "+Mission Loop Brief Anim:" : "+Mission Fork Brief Anim:";
+						if (optional_string_fred(str))
+							parse_comments();
 						else
-							required_string_fred("+Mission Fork Brief Anim:");
-						parse_comments();
-						fout_ext("\n", "%s", link.mission_branch_brief_anim);
-						fout("\n$end_multi_text");
+							fout("\n%s", str);
+
+						if (save_config.save_format != MissionFormat::RETAIL) {
+							fout(" %s", link.mission_branch_brief_anim);
+						} else {
+							// previous formats used multitext, which causes problems
+							fout_ext("\n", "%s", link.mission_branch_brief_anim);
+							fout("\n$end_multi_text");
+						}
 					}
 
 					if ((num_mission_special == 1) && link.mission_branch_brief_sound) {
-						if (mission_loop)
-							required_string_fred("+Mission Loop Brief Sound:");
+						auto str = mission_loop ? "+Mission Loop Brief Sound:" : "+Mission Fork Brief Sound:";
+						if (optional_string_fred(str))
+							parse_comments();
 						else
-							required_string_fred("+Mission Fork Brief Sound:");
-						parse_comments();
-						fout_ext("\n", "%s", link.mission_branch_brief_sound);
-						fout("\n$end_multi_text");
+							fout("\n%s", str);
+
+						if (save_config.save_format != MissionFormat::RETAIL) {
+							fout(" %s", link.mission_branch_brief_sound);
+						} else {
+							// previous formats used multitext, which causes problems
+							fout_ext("\n", "%s", link.mission_branch_brief_sound);
+							fout("\n$end_multi_text");
+						}
 					}
 
 					if (num_mission_special == 1) {

fred2/campaigntreeview.cpp

@@ -32,6 +32,30 @@ campaign_tree_link Links[MAX_CAMPAIGN_TREE_LINKS];
 LOCAL CRect Dragging_rect;
 LOCAL CSize Rect_offset, Last_draw_size;
 
+void init_link(campaign_tree_link &link, int from, int to)
+{
+	link.from = from;
+	link.to = to;
+	link.sexp = Locked_sexp_true;
+	link.node = -1;
+	link.from_pos = -1;
+	link.to_pos = -1;
+	link.is_mission_loop = false;
+	link.is_mission_fork = false;
+	link.mission_branch_txt = nullptr;
+	link.mission_branch_brief_anim = nullptr;
+	link.mission_branch_brief_sound = nullptr;
+	link.p1 = CPoint();
+	link.p2 = CPoint();
+}
+
+void init_element(campaign_tree_element &element)
+{
+	element.from_links = 0;
+	element.to_links = 0;
+	element.box = CRect();
+}
+
 /////////////////////////////////////////////////////////////////////////////
 // campaign_tree_view
 
@@ -247,12 +271,9 @@ void stuff_link_with_formula(int *link_idx, int formula, int mission_num)
 			node = CDR(formula);
 			free_one_sexp(formula);
 			while (node != -1) {
+				init_link(Links[*link_idx], mission_num);
 				node2 = CAR(node);
-				Links[*link_idx].from = mission_num;
 				Links[*link_idx].sexp = CAR(node2);
-				Links[*link_idx].mission_branch_txt = NULL;
-				Links[*link_idx].mission_branch_brief_anim = NULL;
-				Links[*link_idx].mission_branch_brief_sound = NULL;
 				sexp_mark_persistent(CAR(node2));
 				free_one_sexp(node2);
 				node3 = CADR(node2);
@@ -269,7 +290,7 @@ void stuff_link_with_formula(int *link_idx, int formula, int mission_num)
 					}
 
 				} else if ( !stricmp( CTEXT(node3), "end-of-campaign") ) {
-					Links[(*link_idx)++].to = -1;
+					(*link_idx)++;
 					Elements[mission_num].from_links++;
 
 				} else
@@ -292,7 +313,7 @@ void campaign_tree_view::construct_tree()
 
 	// initialize mission link counts
 	for (i=0; i<Campaign.num_missions; i++) {
-		Elements[i].from_links = Elements[i].to_links = 0;
+		init_element(Elements[i]);
 	}
 
 	// analyze branching sexps and build links from them.
@@ -753,14 +774,7 @@ int campaign_tree_view::add_link(int from, int to)
 		return -1;
 
 	Campaign_tree_formp->load_tree(1);
-	Links[Total_links].from = from;
-	Links[Total_links].to = to;
-	Links[Total_links].sexp = Locked_sexp_true;
-	Links[Total_links].is_mission_loop = false;
-	Links[Total_links].is_mission_fork = false;
-	Links[Total_links].mission_branch_txt = NULL;
-	Links[Total_links].mission_branch_brief_anim = NULL;
-	Links[Total_links].mission_branch_brief_sound = NULL;
+	init_link(Links[Total_links], from, to);
 	Total_links++;
 	if (from != to) {
 		if (from >= 0)
@@ -964,7 +978,7 @@ BOOL campaign_tree_view::OnDrop(COleDataObject* pDataObject, DROPEFFECT dropEffe
 		}
 	}
 
-	Elements[Campaign.num_missions].from_links = Elements[Campaign.num_missions].to_links = 0;
+	init_element(Elements[Campaign.num_missions]);
 	cm = &(Campaign.missions[Campaign.num_missions++]);
 	cm->name = strdup(pData);
 	cm->formula = Locked_sexp_true;
@@ -1062,7 +1076,7 @@ void campaign_tree_view::drop_mission(int m, CPoint point)
 		}
 	}
 
-	Elements[Campaign.num_missions].from_links = Elements[Campaign.num_missions].to_links = 0;
+	init_element(Elements[Campaign.num_missions]);
 	cm = &(Campaign.missions[Campaign.num_missions++]);
 	cm->name = strdup(name);
 	cm->formula = Locked_sexp_true;

fred2/campaigntreeview.h

@@ -43,6 +43,9 @@ extern int Sorted[MAX_CAMPAIGN_MISSIONS];
 extern campaign_tree_element Elements[MAX_CAMPAIGN_MISSIONS];
 extern campaign_tree_link Links[MAX_CAMPAIGN_TREE_LINKS];
 
+extern void init_link(campaign_tree_link &link, int from = -1, int to = -1);
+extern void init_element(campaign_tree_element &element);
+
 /////////////////////////////////////////////////////////////////////////////
 // campaign_tree_view view