a few parsing and infrastructure upgrades

- A parse flag to indicate when the mission is being parsed, and a new `ship_class_index()` accessor in the ship registry, both currently unused
- Make the ship registry more robust, especially in FRED: properly update it when ships are renamed or deleted

code/mission/missionparse.cpp

@@ -178,6 +178,9 @@ SCP_vector<SCP_string> Parse_names;
 
 SCP_vector<SCP_string> Mission_parse_warnings;
 
+// true while a mission is being parsed and post-processed
+bool Parsing_mission = false;
+
 // Routes a parse-time auto-correction notice to the right surface for the app:
 // outside QtFRED, the existing Warning(LOCATION, ...) popup; inside QtFRED, the
 // Mission_parse_warnings queue so the ErrorChecker can present it without a popup.
@@ -7552,6 +7555,8 @@ bool parse_main(const char *mission_name, int flags)
 
 	Assert(Ship_info.size() <= MAX_SHIP_CLASSES);
 
+	Parsing_mission = true;
+
 	do {
 		// don't do this for imports
 		if (!(flags & MPF_IMPORT_FSM)) {
@@ -7603,6 +7608,8 @@ bool parse_main(const char *mission_name, int flags)
 		}
 	} while (0);
 
+	Parsing_mission = false;
+
 	if (!Fred_running)
 		strcpy_s(Mission_filename, mission_name);
 

code/mission/missionparse.h

@@ -588,6 +588,9 @@ extern SCP_vector<SCP_string> Parse_names;
 // silently buried. Outside of QtFRED these sites still call Warning(LOCATION, ...).
 extern SCP_vector<SCP_string> Mission_parse_warnings;
 
+// true while a mission is being parsed and post-processed
+extern bool Parsing_mission;
+
 extern char			Player_start_shipname[NAME_LENGTH];
 extern int			Player_start_shipnum;
 extern p_object	*Player_start_pobject;

code/missionui/redalert.cpp

@@ -969,9 +969,8 @@ void red_alert_bash_ship_status()
 					// give the ship its name from the latest wave
 					// (this will make the ship match to the correct red-alert data)
 					wing_bash_ship_name(shipp, wingp, ((rws->latest_wave - 1) * wingp->wave_count) + 1 + pos_in_wing);
-					// need to update the ship registry too
-					strcpy_s(Ship_registry[ship_entry_index].name, shipp->ship_name);
-					Ship_registry_map[shipp->ship_name] = ship_entry_index;
+					// need to update the ship registry too; keep the old key because the previous name was a real ship name that may still be referenced
+					ship_registry_rename(ship_entry_index, shipp->ship_name, false);
 				}
 			}
 		}

code/network/multi_ingame.cpp

@@ -1332,7 +1332,12 @@ void process_ingame_wings_packet( ubyte *data, header *hinfo )
 				// kind of stupid, but bash the name since it won't get recreated properly from
 				// the parse_wing_create_ships call.
 				shipp = &Ships[shipnum];
+				int ship_entry_index = ship_registry_get_index(shipp->ship_name);
+				Assertion(ship_entry_index >= 0, "Ship %s must be in the ship registry!", shipp->ship_name);
 				wing_bash_ship_name(shipp, wingp, which_one + 1);
+				// need to update the ship registry too
+				if (ship_entry_index >= 0)
+					ship_registry_rename(ship_entry_index, shipp->ship_name, true);
 				nprintf(("Network", "Created %s\n", shipp->ship_name));
 
 				objp = &Objects[shipp->objnum];

code/scripting/api/objs/ship.cpp

@@ -343,7 +343,7 @@ ADE_VIRTVAR(ArmorClass, l_Ship, "string", "Current Armor class", "string", "Armo
 	return ade_set_args(L, "s", name);
 }
 
-ADE_VIRTVAR(Name, l_Ship, "string", "Ship name. This is the actual name of the ship. Use <i>getDisplayString</i> to get the string which should be displayed to the player.", "string", "Ship name, or empty string if handle is invalid")
+ADE_VIRTVAR(Name, l_Ship, "string", "Ship name. This is the actual name of the ship. Use <i>getDisplayString</i> to get the string which should be displayed to the player.  Beware of setting the name to the name of an existing ship!", "string", "Ship name, or empty string if handle is invalid")
 {
 	object_h *objh;
 	const char* s = nullptr;
@@ -355,10 +355,18 @@ ADE_VIRTVAR(Name, l_Ship, "string", "Ship name. This is the actual name of the s
 
 	ship *shipp = &Ships[objh->objp()->instance];
 
-	if(ADE_SETTING_VAR && s != nullptr) {
+	if(ADE_SETTING_VAR && s != nullptr)
+	{
+		int ship_entry_index = ship_registry_get_index(shipp->ship_name);
+		Assertion(ship_entry_index >= 0, "Ship %s must be in the ship registry!", shipp->ship_name);
+
 		auto len = sizeof(shipp->ship_name);
 		strncpy(shipp->ship_name, s, len);
 		shipp->ship_name[len - 1] = 0;
+
+		// need to update the ship registry too
+		if (ship_entry_index >= 0)
+			ship_registry_rename(ship_entry_index, shipp->ship_name, true);
 	}
 
 	return ade_set_args(L, "s", shipp->ship_name);

code/ship/ship.cpp

@@ -229,6 +229,19 @@ ship_info* ship_registry_entry::sip() const
 	}
 }
 
+int ship_registry_entry::ship_class_index() const
+{
+	if (shipnum >= 0)
+		return Ships[shipnum].ship_info_index;
+	else if (pobj_num >= 0)
+		return Parse_objects[pobj_num].ship_class;
+	else
+	{
+		Assertion(false, "A ship registry entry must have either a parse object or a ship!");
+		return -1;
+	}
+}
+
 SCP_vector<ship_registry_entry> Ship_registry;
 SCP_unordered_map<SCP_string, int, SCP_string_lcase_hash, SCP_string_lcase_equal_to> Ship_registry_map;
 
@@ -8684,6 +8697,15 @@ void ship_delete( object * obj )
 			shipp->weapons.primary_bank_external_model_instance[i] = -1;
 		}
 	}
+
+	// In FRED, clean up the registry so that stale references don't stick around.  Conversely,
+	// in FSO, we need to keep the registry entry so that ships will still be known in the debriefing.
+	if (Fred_running)
+	{
+		auto ship_it = Ship_registry_map.find(shipp->ship_name);
+		if (ship_it != Ship_registry_map.end())
+			Ship_registry_map.erase(ship_it);	// don't erase the vector entry to avoid clobbering other indexes
+	}
 }
 
 /**

code/ship/ship.h

@@ -1046,6 +1046,7 @@ struct ship_registry_entry
 	ship* shipp_or_null() const;
 
 	ship_info* sip() const;
+	int ship_class_index() const;
 };
 
 extern SCP_vector<ship_registry_entry> Ship_registry;

fred2/freddoc.cpp

@@ -331,15 +331,7 @@ bool CFREDDoc::load_mission(const char *pathname, int flags) {
 				wing_bash_ship_name(name, Wings[i].name, j + 1);
 				old_name = Ships[Wings[i].ship_index[j]].ship_name;
 				if (stricmp(name, old_name) != 0) {  // need to fix name
-					update_sexp_references(old_name, name);
-					ai_update_goal_references(sexp_ref_type::SHIP, old_name, name);
-					update_texture_replacements(old_name, name);
-					int k = find_item_with_string(Reinforcements, &reinforcements::name, old_name);
-					if (k >= 0) {
-						Assert(strlen(name) < NAME_LENGTH);
-						strcpy_s(Reinforcements[k].name, name);
-					}
-
+					rename_ship(Wings[i].ship_index[j], name);
 					// bash it again so that we handle display names if needed
 					wing_bash_ship_name(&Ships[Wings[i].ship_index[j]], &Wings[i], j + 1, true);
 				}

fred2/fredview.cpp

@@ -1185,6 +1185,10 @@ void CFREDView::OnLButtonUp(UINT nFlags, CPoint point)
 						Assert(objp->type == OBJ_SHIP);
 						ship = objp->instance;
 						Assert(Ships[ship].wingnum == -1);
+						char new_name[NAME_LENGTH];
+						wing_bash_ship_name(new_name, Wings[Duped_wing].name, Wings[Duped_wing].wave_count + 1);
+						rename_ship(ship, new_name);
+						// bash it again for the display name
 						wing_bash_ship_name(&Ships[ship], &Wings[Duped_wing], Wings[Duped_wing].wave_count + 1, true);
 
 						Wings[Duped_wing].ship_index[Wings[Duped_wing].wave_count] = ship;

fred2/management.cpp

@@ -532,11 +532,40 @@ void fix_prop_name(int prop)
 
 void fix_ship_name(int ship)
 {
+	char old_name[NAME_LENGTH];
+	strcpy_s(old_name, Ships[ship].ship_name);
+
 	int i = 1;
 
 	do {
 		sprintf(Ships[ship].ship_name, "U.R.A. Moron %d", i++);
 	} while (query_ship_name_duplicate(ship));
+
+	// This function is called when a newly created ship duplicates the name of an existing ship.  In
+	// that situation, ship_create() will have overwritten the existing ship's registry entry to point
+	// to the new ship, so point it back at the ship that legitimately holds the old name.
+	auto ship_it = Ship_registry_map.find(old_name);
+	if (ship_it != Ship_registry_map.end() && Ship_registry[ship_it->second].shipnum == ship)
+	{
+		int other_shipnum = ship_name_lookup(old_name, 1);
+		if (other_shipnum >= 0)
+		{
+			auto old_entry = &Ship_registry[ship_it->second];
+			old_entry->objnum = Ships[other_shipnum].objnum;
+			old_entry->shipnum = other_shipnum;
+		}
+		else
+			Ship_registry_map.erase(ship_it);	// don't erase the vector entry to avoid clobbering other indexes
+	}
+
+	// add a fresh registry entry for this ship under its new name
+	ship_registry_entry entry(Ships[ship].ship_name);
+	entry.status = ShipStatus::PRESENT;
+	entry.objnum = Ships[ship].objnum;
+	entry.shipnum = ship;
+
+	Ship_registry.push_back(entry);
+	Ship_registry_map[Ships[ship].ship_name] = sz2i(Ship_registry.size() - 1);
 }
 
 int create_ship(matrix *orient, vec3d *pos, int ship_type)
@@ -1817,7 +1846,7 @@ int get_docking_list(int model_index)
 }
 
 // DA 1/7/99 These ship names are not variables
-int rename_ship(int ship, const char *name)
+int rename_ship(int ship, const char *name, bool update_display_name)
 {
 	Assert(ship >= 0);
 	Assert(strlen(name) < NAME_LENGTH);
@@ -1835,35 +1864,34 @@ int rename_ship(int ship, const char *name)
 
 	// keep the ship registry in sync
 	auto reg_it = Ship_registry_map.find(Ships[ship].ship_name);
-	if (reg_it != Ship_registry_map.end()) {
-		int reg_idx = reg_it->second;
-		Ship_registry_map.erase(reg_it);
-		strcpy_s(Ship_registry[reg_idx].name, name);
-		Ship_registry_map[name] = reg_idx;
-	}
+	if (reg_it != Ship_registry_map.end())
+		ship_registry_rename(reg_it->second, name, true);
 
 	strcpy_s(Ships[ship].ship_name, name);
 	if (ship == cur_ship)
 		Ship_editor_dialog.m_ship_name = _T(name);
 
-	// if this name has a hash, create a default display name
-	if (get_pointer_to_first_hash_symbol(Ships[ship].ship_name))
+	if (update_display_name)
 	{
-		Ships[ship].display_name = Ships[ship].ship_name;
-		end_string_at_first_hash_symbol(Ships[ship].display_name);
-		Ships[ship].flags.set(Ship::Ship_Flags::Has_display_name);
+		// if this name has a hash, create a default display name
+		if (get_pointer_to_first_hash_symbol(Ships[ship].ship_name))
+		{
+			Ships[ship].display_name = Ships[ship].ship_name;
+			end_string_at_first_hash_symbol(Ships[ship].display_name);
+			Ships[ship].flags.set(Ship::Ship_Flags::Has_display_name);
 
-		if (ship == cur_ship)
-			Ship_editor_dialog.m_ship_display_name = _T(Ships[ship].display_name.c_str());
-	}
-	// otherwise reset the display name
-	else
-	{
-		Ships[ship].display_name = "";
-		Ships[ship].flags.remove(Ship::Ship_Flags::Has_display_name);
+			if (ship == cur_ship)
+				Ship_editor_dialog.m_ship_display_name = _T(Ships[ship].display_name.c_str());
+		}
+		// otherwise reset the display name
+		else
+		{
+			Ships[ship].display_name = "";
+			Ships[ship].flags.remove(Ship::Ship_Flags::Has_display_name);
 
-		if (ship == cur_ship)
-			Ship_editor_dialog.m_ship_display_name = _T("<none>");
+			if (ship == cur_ship)
+				Ship_editor_dialog.m_ship_display_name = _T("<none>");
+		}
 	}
 
 	return 0;

fred2/management.h

@@ -98,7 +98,7 @@ int query_initial_orders_conflict(int wing);
 int query_initial_orders_empty(ai_goal* ai_goals);
 int set_reinforcement(const char* name, int state);
 int get_docking_list(int model_index);
-int rename_ship(int ship, const char* name);
+int rename_ship(int ship, const char* name, bool update_display_name = true);
 void fix_ship_name(int ship);
 int internal_integrity_check();
 void correct_marking();

fred2/shipeditordlg.cpp

@@ -1063,7 +1063,7 @@ void CShipEditorDlg::initialize_data(int full_update)
 // Once the error no longer occurs, bypass mode is cleared and data is updated.
 int CShipEditorDlg::update_data(int redraw)
 {
-	char *str, old_name[255];
+	char old_name[255];
 	object *ptr;
 	int i, z, wing;
 	CSingleLock sync(&CS_cur_object_index), sync2(&CS_update);
@@ -1136,17 +1136,11 @@ int CShipEditorDlg::update_data(int redraw)
 		if (z)
 			return z;
 
-		strcpy_s(old_name, Ships[single_ship].ship_name);
-		string_copy(Ships[single_ship].ship_name, m_ship_name, NAME_LENGTH - 1, 1);
-		str = Ships[single_ship].ship_name;
-		if (strcmp(old_name, str)) {
-			update_sexp_references(old_name, str);
-			ai_update_goal_references(sexp_ref_type::SHIP, old_name, str);
-			update_texture_replacements(old_name, str);
-			i = find_item_with_string(Reinforcements, &reinforcements::name, old_name);
-			if (i >= 0)
-				strcpy_s(Reinforcements[i].name, str);
-
+		char new_name[NAME_LENGTH];
+		string_copy(new_name, m_ship_name, NAME_LENGTH - 1, 1);
+		if (strcmp(Ships[single_ship].ship_name, new_name) != 0) {
+			// the display name was already handled in update_ship
+			rename_ship(single_ship, new_name, false);
 			Update_window = 1;
 		}
 	}

qtfred/src/mission/Editor.cpp

@@ -337,15 +337,7 @@ bool Editor::loadMission(const std::string& mission_name, int flags) {
 				wing_bash_ship_name(name, Wings[i].name, j + 1);
 				old_name = Ships[Wings[i].ship_index[j]].ship_name;
 				if (stricmp(name, old_name) != 0) {  // need to fix name
-					update_sexp_references(old_name, name);
-					ai_update_goal_references(sexp_ref_type::SHIP, old_name, name);
-					update_texture_replacements(old_name, name);
-					int k = find_item_with_string(Reinforcements, &reinforcements::name, old_name);
-					if (k >= 0) {
-						Assert(strlen(name) < NAME_LENGTH);
-						strcpy_s(Reinforcements[k].name, name);
-					}
-
+					rename_ship(Wings[i].ship_index[j], name);
 					// bash it again so that we handle display names if needed
 					wing_bash_ship_name(&Ships[Wings[i].ship_index[j]], &Wings[i], j + 1, true);
 				}
@@ -805,11 +797,40 @@ bool Editor::query_ship_name_duplicate(int ship) {
 }
 
 void Editor::fix_ship_name(int ship) {
+	char old_name[NAME_LENGTH];
+	strcpy_s(old_name, Ships[ship].ship_name);
+
 	int i = 1;
 
 	do {
 		sprintf(Ships[ship].ship_name, "U.R.A. Moron %d", i++);
 	} while (query_ship_name_duplicate(ship));
+
+	// This function is called when a newly created ship duplicates the name of an existing ship.  In
+	// that situation, ship_create() will have overwritten the existing ship's registry entry to point
+	// to the new ship, so point it back at the ship that legitimately holds the old name.
+	auto ship_it = Ship_registry_map.find(old_name);
+	if (ship_it != Ship_registry_map.end() && Ship_registry[ship_it->second].shipnum == ship)
+	{
+		int other_shipnum = ship_name_lookup(old_name, 1);
+		if (other_shipnum >= 0)
+		{
+			auto old_entry = &Ship_registry[ship_it->second];
+			old_entry->objnum = Ships[other_shipnum].objnum;
+			old_entry->shipnum = other_shipnum;
+		}
+		else
+			Ship_registry_map.erase(ship_it);	// don't erase the vector entry to avoid clobbering other indexes
+	}
+
+	// add a fresh registry entry for this ship under its new name
+	ship_registry_entry entry(Ships[ship].ship_name);
+	entry.status = ShipStatus::PRESENT;
+	entry.objnum = Ships[ship].objnum;
+	entry.shipnum = ship;
+
+	Ship_registry.push_back(entry);
+	Ship_registry_map[Ships[ship].ship_name] = sz2i(Ship_registry.size() - 1);
 }
 
 void Editor::createNewMission() {
@@ -1434,7 +1455,7 @@ void Editor::update_texture_replacements(const char* old_name, const char* new_n
 			strcpy_s(ii->ship_name, new_name);
 	}
 }
-int Editor::rename_ship(int ship, const char* name) {
+int Editor::rename_ship(int ship, const char* name, bool update_display_name) {
 	Assert(ship >= 0);
 	Assert(strlen(name) < NAME_LENGTH);
 
@@ -1451,27 +1472,26 @@ int Editor::rename_ship(int ship, const char* name) {
 
 	// keep the ship registry in sync
 	auto reg_it = Ship_registry_map.find(Ships[ship].ship_name);
-	if (reg_it != Ship_registry_map.end()) {
-		int reg_idx = reg_it->second;
-		Ship_registry_map.erase(reg_it);
-		strcpy_s(Ship_registry[reg_idx].name, name);
-		Ship_registry_map[name] = reg_idx;
-	}
+	if (reg_it != Ship_registry_map.end())
+		ship_registry_rename(reg_it->second, name, true);
 
 	strcpy_s(Ships[ship].ship_name, name);
 
-	// if this name has a hash, create a default display name
-	if (get_pointer_to_first_hash_symbol(Ships[ship].ship_name))
+	if (update_display_name)
 	{
-		Ships[ship].display_name = Ships[ship].ship_name;
-		end_string_at_first_hash_symbol(Ships[ship].display_name);
-		Ships[ship].flags.set(Ship::Ship_Flags::Has_display_name);
-	}
-	// otherwise reset the display name
-	else
-	{
-		Ships[ship].display_name = "";
-		Ships[ship].flags.remove(Ship::Ship_Flags::Has_display_name);
+		// if this name has a hash, create a default display name
+		if (get_pointer_to_first_hash_symbol(Ships[ship].ship_name))
+		{
+			Ships[ship].display_name = Ships[ship].ship_name;
+			end_string_at_first_hash_symbol(Ships[ship].display_name);
+			Ships[ship].flags.set(Ship::Ship_Flags::Has_display_name);
+		}
+		// otherwise reset the display name
+		else
+		{
+			Ships[ship].display_name = "";
+			Ships[ship].flags.remove(Ship::Ship_Flags::Has_display_name);
+		}
 	}
 
 	missionChanged();

qtfred/src/mission/Editor.h

@@ -221,7 +221,7 @@ class Editor : public QObject {
 	bool rename_wing(int wing, const SCP_string& new_name, bool rename_members = true);
 
 	// DA 1/7/99 These ship names are not variables
-	int rename_ship(int ship, const char* name);
+	int rename_ship(int ship, const char* name, bool update_display_name = true);
 
 	/**
 	 * @brief Delete a whole wing, leaving ships intact but wingless.

qtfred/src/mission/dialogs/ShipEditor/ShipEditorDialogModel.cpp

@@ -772,22 +772,10 @@ void ShipEditorDialogModel::setShipName(const SCP_string& m_ship_name)
 	}
 
 	// All validation passed — write the new name
-	char old_name[NAME_LENGTH];
-	strcpy_s(old_name, Ships[_singleShip].ship_name);
-	strcpy_s(Ships[_singleShip].ship_name, new_name.c_str());
+	// (the display name is handled separately in setShipDisplayName)
+	_editor->rename_ship(_singleShip, new_name.c_str(), false);
 	_shipName = new_name;
 
-	if (strcmp(old_name, Ships[_singleShip].ship_name)) {
-		update_sexp_references(old_name, Ships[_singleShip].ship_name);
-		_editor->ai_update_goal_references(sexp_ref_type::SHIP, old_name, Ships[_singleShip].ship_name);
-		_editor->update_texture_replacements(old_name, Ships[_singleShip].ship_name);
-		int j = find_item_with_string(Reinforcements, &reinforcements::name, old_name);
-		if (j >= 0) {
-			Assert(strlen(Ships[_singleShip].ship_name) < NAME_LENGTH);
-			strcpy_s(Reinforcements[j].name, Ships[_singleShip].ship_name);
-		}
-	}
-
 	setModified();
 	_editor->missionChanged();
 	modelChanged();

qtfred/src/ui/widgets/renderwidget.cpp

@@ -476,6 +476,10 @@ void RenderWidget::mouseReleaseEvent(QMouseEvent* event) {
 						Assert(objp->type == OBJ_SHIP);
 						ship = objp->instance;
 						Assert(Ships[ship].wingnum == -1);
+						char new_name[NAME_LENGTH];
+						wing_bash_ship_name(new_name, Wings[_viewport->Duped_wing].name, Wings[_viewport->Duped_wing].wave_count + 1);
+						fred->rename_ship(ship, new_name);
+						// bash it again for the display name
 						wing_bash_ship_name(&Ships[ship], &Wings[_viewport->Duped_wing], Wings[_viewport->Duped_wing].wave_count + 1, true);
 
 						Wings[_viewport->Duped_wing].ship_index[Wings[_viewport->Duped_wing].wave_count] = ship;