Merge pull request scp-fs2open#7600 from Goober5000/fix/ship_demotion

fix a bug with player ships being demoted to common ships

Author: Goober5000

code/missioneditor/common.cpp

@@ -1,7 +1,9 @@
 // methods and members common to any mission editor FSO may have
 #include "common.h"
+#include "globalincs/linklist.h"
 #include "mission/missionparse.h"
 #include "iff_defs/iff_defs.h"
+#include "object/object.h"
 #include "ship/ship.h"
 
 // to keep track of data
@@ -145,3 +147,22 @@ void generate_weaponry_usage_list_wing(int wing_num, int* arr)
 		}
 	}
 }
+
+void ensure_valid_player_start_shipnum()
+{
+	// nothing to do if the current player start is still valid
+	if (Player_start_shipnum >= 0 && Player_start_shipnum < MAX_SHIPS
+		&& Ships[Player_start_shipnum].objnum >= 0
+		&& Objects[Ships[Player_start_shipnum].objnum].type == OBJ_START) {
+		return;
+	}
+
+	// otherwise repoint to the first remaining player start, or -1 if there are none
+	Player_start_shipnum = -1;
+	for (auto *objp : list_range(&obj_used_list)) {
+		if (objp->type == OBJ_START) {
+			Player_start_shipnum = objp->instance;
+			break;
+		}
+	}
+}

code/missioneditor/common.h

@@ -48,3 +48,8 @@ anchor_t target_to_anchor(int target);
 void generate_weaponry_usage_list_team(int team, int* arr);
 
 void generate_weaponry_usage_list_wing(int wing_num, int* arr);
+
+// If Player_start_shipnum no longer refers to a valid player start ship, repoint it to the
+// first remaining player start in the mission (or -1 if there are none).  Call this after
+// changing a ship to or from an OBJ_START via demotion, deletion, etc.
+void ensure_valid_player_start_shipnum();

code/missioneditor/missionsave.cpp

@@ -4393,8 +4393,7 @@ int Fred_mission_save::save_players()
 	for (i = 0; i < Num_teams; i++) {
 		required_string_fred("$Starting Shipname:");
 		parse_comments();
-		Assert(Player_start_shipnum >= 0);
-		fout(" %s", Ships[Player_start_shipnum].ship_name);
+		fout(" %s", (Player_start_shipnum >= 0) ? Ships[Player_start_shipnum].ship_name : "<none>");
 
 		if (save_config.save_format != MissionFormat::RETAIL) {
 			if (Team_data[i].do_not_validate) {

fred2/fredview.cpp

@@ -32,6 +32,7 @@
 #include "ai/aigoals.h"
 #include "ship/ship.h"	// for ship names
 #include "prop/prop.h" // for prop names
+#include "missioneditor/common.h"
 #include "MissionGoalsDlg.h"
 #include "MissionCutscenesDlg.h"
 #include "wing.h"
@@ -2641,6 +2642,9 @@ int CFREDView::global_error_check()
 					ptr->type = OBJ_SHIP;
 					Player_starts--;
 					t--;
+
+					ensure_valid_player_start_shipnum();
+
 					if (error("Invalid ship type for a player.  Ship has been reset to non-player ship.")){
 						return 1;
 					}
@@ -3130,7 +3134,9 @@ int CFREDView::global_error_check()
 				return -1;
 	}*/
 
-	Assert((Player_start_shipnum >= 0) && (Player_start_shipnum < MAX_SHIPS) && (Ships[Player_start_shipnum].objnum >= 0));
+	if ((Player_start_shipnum < 0) || (Player_start_shipnum >= MAX_SHIPS) || (Ships[Player_start_shipnum].objnum < 0)){
+		return internal_error("Mission has no valid player start ship");
+	}
 	i = global_error_check_player_wings(multi);
 	if (i){
 		return i;

fred2/management.cpp

@@ -1235,7 +1235,6 @@ int common_object_delete(int obj)
 	char msg[255];
 	const char *name;
 	int i, z, r, type;
-	object *objp;
 	SCP_list<CJumpNode>::iterator jnp;
 
 	type = Objects[obj].type;
@@ -1271,17 +1270,7 @@ int common_object_delete(int obj)
 			ai_do_objects_undocked_stuff(&Objects[obj], dock_get_first_docked_object(&Objects[obj]));
 		}
 
-		if (Player_start_shipnum == i) {  // need a new single player start.
-			objp = GET_FIRST(&obj_used_list);
-			while (objp != END_OF_LIST(&obj_used_list)) {
-				if (objp->type == OBJ_START) {
-					Player_start_shipnum = objp->instance;
-					break;
-				}
-
-				objp = GET_NEXT(objp);
-			}
-		}
+		ensure_valid_player_start_shipnum();  // may need a new single player start
 
 		Player_starts--;
 

fred2/shipeditordlg.cpp

@@ -1258,17 +1258,7 @@ int CShipEditorDlg::update_data(int redraw)
 		}
 	}
 
-	if (Player_start_shipnum < 0 || Objects[Ships[Player_start_shipnum].objnum].type != OBJ_START) {  // need a new single player start.
-		ptr = GET_FIRST(&obj_used_list);
-		while (ptr != END_OF_LIST(&obj_used_list)) {
-			if (ptr->type == OBJ_START) {
-				Player_start_shipnum = ptr->instance;
-				break;
-			}
-
-			ptr = GET_NEXT(ptr);
-		}
-	}
+	ensure_valid_player_start_shipnum();  // may need a new single player start
 
 	if (modified)
 		set_modified();
@@ -1518,7 +1508,9 @@ int CShipEditorDlg::update_ship(int ship)
 
 			Objects[Ships[ship].objnum].type = OBJ_SHIP;
 			break;
-	}	
+	}
+
+	ensure_valid_player_start_shipnum();
 
 	Update_ship = 1;
 	return 0;
@@ -2448,19 +2440,32 @@ void CShipEditorDlg::OnSetAsPlayerShip()
 			if (objp->flags[Object::Object_Flags::Marked])	// there should only be one selected ship
 			{
 				// set as player ship
+				if (objp->type != OBJ_START)
+				{
+					Player_starts++;
+					set_modified();
+				}
 				objp->type = OBJ_START;
 				objp->flags.set(Object::Object_Flags::Player_ship);
 			}
 			else
 			{
 				// set as regular ship
+				if (objp->type == OBJ_START)
+				{
+					Player_starts--;
+					set_modified();
+				}
 				objp->type = OBJ_SHIP;
 				objp->flags.remove(Object::Object_Flags::Player_ship);
 			}
 		}
 		objp = GET_NEXT(objp);
 	}
 
+	// fix up Player_start_shipnum if it became invalid
+	ensure_valid_player_start_shipnum();
+
 	// finally set editor dialog
 	m_player_ship.SetCheck(1);
 	update_map_window();

fred2/wing.cpp

@@ -227,7 +227,8 @@ int create_wing() {
 	}
 
 	count = 0;
-	if (Objects[Ships[Player_start_shipnum].objnum].flags[Object::Object_Flags::Marked])
+	if ((Player_start_shipnum >= 0) && (Player_start_shipnum < MAX_SHIPS) && (Ships[Player_start_shipnum].objnum >= 0)
+		&& Objects[Ships[Player_start_shipnum].objnum].flags[Object::Object_Flags::Marked])
 		count = 1;
 
 	ptr = GET_FIRST(&obj_used_list);

qtfred/src/mission/Editor.cpp

@@ -974,7 +974,6 @@ int Editor::common_object_delete(int obj) {
 	char msg[255];
 	const char *name;
 	int i, z, r, type;
-	object* objp;
 	SCP_list<CJumpNode>::iterator jnp;
 
 	type = Objects[obj].type;
@@ -1013,17 +1012,7 @@ int Editor::common_object_delete(int obj) {
 			ai_do_objects_undocked_stuff(&Objects[obj], dock_get_first_docked_object(&Objects[obj]));
 		}
 
-		if (Player_start_shipnum == i) {  // need a new single player start.
-			objp = GET_FIRST(&obj_used_list);
-			while (objp != END_OF_LIST(&obj_used_list)) {
-				if (objp->type == OBJ_START) {
-					Player_start_shipnum = objp->instance;
-					break;
-				}
-
-				objp = GET_NEXT(objp);
-			}
-		}
+		ensure_valid_player_start_shipnum();  // may need a new single player start
 
 		Player_starts--;
 

qtfred/src/mission/EditorWing.cpp

@@ -240,7 +240,8 @@ int Editor::create_wing()
 	}
 
 	count = 0;
-	if (Objects[Ships[Player_start_shipnum].objnum].flags[Object::Object_Flags::Marked]) {
+	if ((Player_start_shipnum >= 0) && (Player_start_shipnum < MAX_SHIPS) && (Ships[Player_start_shipnum].objnum >= 0)
+		&& Objects[Ships[Player_start_shipnum].objnum].flags[Object::Object_Flags::Marked]) {
 		count = 1;
 	}
 

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

@@ -1150,15 +1150,8 @@ void ShipEditorDialogModel::setPlayer(const bool m_player)
 			}
 		}
 	}
-	// Fix up Player_start_shipnum if it became invalid
-	if (Player_start_shipnum < 0 || Objects[Ships[Player_start_shipnum].objnum].type != OBJ_START) {
-		for (auto* p = GET_FIRST(&obj_used_list); p != END_OF_LIST(&obj_used_list); p = GET_NEXT(p)) {
-			if (p->type == OBJ_START) {
-				Player_start_shipnum = p->instance;
-				break;
-			}
-		}
-	}
+	// fix up Player_start_shipnum if it became invalid
+	ensure_valid_player_start_shipnum();
 	setModified();
 	_editor->missionChanged();
 	modelChanged();