fix ShipStatus states to correctly account for exploding ships

Adds a `DEATH_ROLL` status to specifically cover the state after ships have been destroyed but before they exit.  I've also audited all code that interacts with the ship registry to use the correct state with this new state taken into consideration.

This fixes access to exploding ships that was broken when the ship registry framework was introduced.  In particular, the `when destroyed X beam-lock-all X` QoL feature now works again.

code/ai/aicode.cpp

@@ -386,7 +386,7 @@ int is_preferred_weapon(int weapon_num, object *firer_objp, object *target_objp)
 			continue;
 
 		auto ship_entry = &Ship_registry[hfi->ship_registry_index];
-		if (ship_entry->status != ShipStatus::PRESENT)
+		if (!ship_entry->objp)
 			continue;
 
 		int signature = ship_entry->objp->signature;

code/ai/aigoals.cpp

@@ -976,7 +976,7 @@ void ai_add_goal_sub_sexp( int sexp, int type, ai_info *aip, ai_goal *aigp, char
 		// this goal needs some extra setup
 		// if this doesn't work, the goal will be immediately removed
 		auto ship_entry = ship_registry_get(aigp->target_name);
-		if (ship_entry->status == ShipStatus::PRESENT)
+		if (ship_entry->shipp)
 		{
 			auto target_aip = &Ai_info[ship_entry->shipp->ai_index];
 			target_aip->ai_flags.set(AI::AI_Flags::Awaiting_repair);
@@ -1797,7 +1797,7 @@ ai_achievability ai_mission_goal_achievable( int objnum, ai_goal *aigp )
 			status = SHIP_STATUS_UNKNOWN;
 		}
 		// goal ship is currently in mission
-		else if (ship_entry->status == ShipStatus::PRESENT)
+		else if (ship_entry->status == ShipStatus::PRESENT || ship_entry->status == ShipStatus::DEATH_ROLL)
 		{
 			status = SHIP_STATUS_ARRIVED;
 		}

code/mission/missiontraining.cpp

@@ -922,7 +922,7 @@ void message_training_queue_check()
 	auto ship_entry = ship_registry_get(NOX("instructor"));
 	if (ship_entry != nullptr)
 	{
-		if (ship_entry->status == ShipStatus::PRESENT)
+		if (ship_entry->shipp != nullptr)
 		{
 			// if the instructor is dying or departing, do nothing
 			if (ship_entry->shipp->is_dying_or_departing())

code/parse/sexp.cpp

@@ -6585,6 +6585,7 @@ void eval_object_ship_wing_point_team(object_ship_wing_point_team *oswpt, int no
 				break;
 
 			case ShipStatus::PRESENT:
+			case ShipStatus::DEATH_ROLL:
 				oswpt->type = OSWPT_TYPE_SHIP;
 				break;
 
@@ -6853,7 +6854,7 @@ int sexp_is_destroyed(int n, fix *latest_time)
 			if (ship_entry->status == ShipStatus::NOT_YET_PRESENT)
 				return SEXP_CANT_EVAL;
 
-			if (ship_entry->status == ShipStatus::EXITED)
+			if (ship_entry->status == ShipStatus::DEATH_ROLL || ship_entry->status == ShipStatus::EXITED)
 			{
 				// check the mission log
 				if (mission_log_get_time(LOG_SHIP_DESTROYED, ship_entry->name, nullptr, &time) || mission_log_get_time(LOG_SELF_DESTRUCTED, ship_entry->name, nullptr, &time))
@@ -7301,7 +7302,7 @@ int sexp_was_destroyed_by(int n, fix *latest_time)
 				*latest_time = time;
 		}
 		// if the ship has exited, no way to destroy it
-		else if (ship_entry->status == ShipStatus::EXITED)
+		else if (destroyed_ship_entry->status == ShipStatus::EXITED)
 			return SEXP_KNOWN_FALSE;
 	}
 
@@ -7526,7 +7527,14 @@ int sexp_time_exited(int n, int op_num)
 	if (ship)
 	{
 		auto ship_entry = eval_ship(n);
-		if (!ship_entry || ship_entry->status != ShipStatus::EXITED)
+		if (!ship_entry)
+			return SEXP_NAN;
+		if (ship_entry->status == ShipStatus::DEATH_ROLL)
+		{
+			if (!destroyed)
+				return SEXP_NAN;
+		}
+		else if (ship_entry->status != ShipStatus::EXITED)
 			return SEXP_NAN;
 
 		if (destroyed)
@@ -8784,7 +8792,8 @@ int sexp_angle_vectors(int node) {
 	return fl2ir(angle);
 }
 
-int sexp_angle_fvec_target(int node) {
+int sexp_angle_fvec_target(int node)
+{
 	auto* ship = eval_ship(node);
 
 	if (ship->status == ShipStatus::EXITED)
@@ -9412,7 +9421,7 @@ int sexp_last_order_time(int n)
 
 		auto ship_entry = eval_ship(n);
 		if (ship_entry) {
-			if (ship_entry->status == ShipStatus::PRESENT) {
+			if (ship_entry->shipp) {
 				aigp = Ai_info[ship_entry->shipp->ai_index].goals;
 			}
 		} else {
@@ -9785,7 +9794,7 @@ int sexp_destroyed_departed_delay(int n)
 					return SEXP_CANT_EVAL;
 				}
 
-				if (ship_entry->status == ShipStatus::EXITED) {
+				if (ship_entry->status == ShipStatus::DEATH_ROLL || ship_entry->status == ShipStatus::EXITED) {
 					if (mission_log_get_time(LOG_SHIP_DEPARTED, ship_entry->name, nullptr, &time_gone)) {
 						count++;
 					} else if (mission_log_get_time(LOG_SHIP_DESTROYED, ship_entry->name, nullptr, &time_gone)) {
@@ -11847,6 +11856,8 @@ int sexp_is_iff(int n)
 					// ship is in the EXITED state but probably in the process of exploding
 					else if (oswpt.ship_entry->shipp)
 					{
+						UNREACHABLE("With the addition of the ShipStatus::DEATH_ROLL state, this shouldn't happen");
+
 						// if the team doesn't match the team specified, return false immediately
 						if (oswpt.ship_entry->shipp->team != team)
 							return SEXP_KNOWN_FALSE;
@@ -12335,7 +12346,7 @@ void sexp_add_goal(int n)
 	auto ship_entry = eval_ship(n);
 	if (ship_entry)
 	{
-		if (ship_entry->status != ShipStatus::PRESENT)
+		if (!ship_entry->shipp)
 			return;										// ship not around anymore???? then forget it!
 
 		ai_add_ship_goal_sexp(goal_node, AIG_TYPE_EVENT_SHIP, &(Ai_info[ship_entry->shipp->ai_index]));
@@ -12362,7 +12373,7 @@ void sexp_remove_goal(int n)
 	auto ship_entry = eval_ship(n);
 	if (ship_entry)
 	{
-		if (ship_entry->status != ShipStatus::PRESENT)
+		if (!ship_entry->shipp)
 			return;										// ship not around anymore???? then forget it!
 
 		int goalindex = ai_remove_goal_sexp_sub(goal_node, Ai_info[ship_entry->shipp->ai_index].goals);
@@ -12394,7 +12405,7 @@ void sexp_clear_goals(int n)
 		auto ship_entry = eval_ship(n);
 		if (ship_entry)
 		{
-			if (ship_entry->status != ShipStatus::PRESENT)
+			if (!ship_entry->shipp)
 				continue;										// ship not around anymore???? then forget it!
 
 			ai_clear_ship_goals(&(Ai_info[ship_entry->shipp->ai_index]));
@@ -16225,7 +16236,7 @@ void sexp_deal_with_ship_flag(int node, bool process_subsequent_nodes, Object::O
 			continue;
 
 		// if ship is in-mission
-		if (ship_entry->status == ShipStatus::PRESENT)
+		if (ship_entry->shipp)
 		{
 			// save flags for state change comparisons
 			auto object_flag_orig = ship_entry->objp->flags;
@@ -20181,23 +20192,20 @@ void sexp_set_alphamult(int n)
 		case OSWPT_TYPE_SHIP:
 		{
 			auto ship_entry = oswpt.ship_entry;
+			ship* shipp = ship_entry->shipp;
 
-			if (ship_entry->status == ShipStatus::PRESENT)
-			{
-				ship* shipp = ship_entry->shipp;
-				shipp->flags.remove(Ship::Ship_Flags::Cloaked);
-				if (newAlpha >= 100) {
-					shipp->alpha_mult = 1.0f;
-					shipp->flags.remove(Ship::Ship_Flags::Render_with_alpha_mult);
-				}
-				else if (newAlpha <= 0) {
-					shipp->alpha_mult = 0.0f;
-					shipp->flags.set(Ship::Ship_Flags::Cloaked);
-				}
-				else {
-					shipp->alpha_mult = ((float)newAlpha) / 100.f;
-					shipp->flags.set(Ship::Ship_Flags::Render_with_alpha_mult);
-				}
+			shipp->flags.remove(Ship::Ship_Flags::Cloaked);
+			if (newAlpha >= 100) {
+				shipp->alpha_mult = 1.0f;
+				shipp->flags.remove(Ship::Ship_Flags::Render_with_alpha_mult);
+			}
+			else if (newAlpha <= 0) {
+				shipp->alpha_mult = 0.0f;
+				shipp->flags.set(Ship::Ship_Flags::Cloaked);
+			}
+			else {
+				shipp->alpha_mult = ((float)newAlpha) / 100.f;
+				shipp->flags.set(Ship::Ship_Flags::Render_with_alpha_mult);
 			}
 			break;
 		}
@@ -20307,7 +20315,7 @@ void sexp_ship_copy_damage(int node)
 			continue;
 
 		// maybe it's present in-mission
-		if (target->status == ShipStatus::PRESENT)
+		if (target->shipp)
 		{
 			ship_copy_damage(target->shipp, source->shipp);
 			continue;
@@ -25709,7 +25717,7 @@ int sexp_is_in_mission(int node)
 	{
 		// For this sexp, we do not short-circuit known-true or known-false.
 		auto ship_entry = eval_ship(n);
-		if (!ship_entry || ship_entry->status != ShipStatus::PRESENT)
+		if (!ship_entry || !ship_entry->shipp)
 			return SEXP_FALSE;
 	}
 
@@ -25718,7 +25726,6 @@ int sexp_is_in_mission(int node)
 
 int sexp_has_armor_type(int node)
 {
-
 	// get ship from sexp
 	auto ship_entry = eval_ship(node);
 	if (!ship_entry || ship_entry->status == ShipStatus::NOT_YET_PRESENT)
@@ -25808,7 +25815,7 @@ void sexp_manipulate_colgroup(int node, bool add_to_group)
 		return;
 	node = CDR(node);
 
-	int colgroup_id = (ship_entry->status == ShipStatus::PRESENT)
+	int colgroup_id = (ship_entry->objp)
 		? ship_entry->objp->collision_group_id
 		: ship_entry->p_objp->collision_group_id;
 
@@ -25830,7 +25837,7 @@ void sexp_manipulate_colgroup(int node, bool add_to_group)
 		}
 	}
 
-	if (ship_entry->status == ShipStatus::PRESENT)
+	if (ship_entry->objp)
 		ship_entry->objp->collision_group_id = colgroup_id;
 	else
 		ship_entry->p_objp->collision_group_id = colgroup_id;
@@ -25856,7 +25863,7 @@ void sexp_manipulate_colgroup_new(int node, bool add_to_group)
 		if (!ship_entry || ship_entry->status == ShipStatus::EXITED)
 			continue;
 
-		if (ship_entry->status == ShipStatus::PRESENT)
+		if (ship_entry->objp)
 		{
 			if (add_to_group)
 				ship_entry->objp->collision_group_id |= (1 << group);
@@ -25881,7 +25888,7 @@ int sexp_get_colgroup(int node)
 	if (ship_entry->status == ShipStatus::EXITED)
 		return SEXP_NAN_FOREVER;
 
-	if (ship_entry->status == ShipStatus::PRESENT)
+	if (ship_entry->objp)
 		return ship_entry->objp->collision_group_id;
 	else
 		return ship_entry->p_objp->collision_group_id;

code/parse/sexp/LuaSEXP.cpp

@@ -176,7 +176,7 @@ luacpp::LuaValue LuaSEXP::sexpToLua(int node, int argnum, int parent_node) const
 			return LuaValue::createValue(_action.getLuaState(), ship_entry ? ship_entry->name : "");
 		}
 
-		if (!ship_entry || ship_entry->status != ShipStatus::PRESENT) {
+		if (!ship_entry || !ship_entry->objp) {
 			// Name is invalid
 			return LuaValue::createValue(_action.getLuaState(), l_Ship.Set(object_h()));
 		}
@@ -256,7 +256,7 @@ luacpp::LuaValue LuaSEXP::sexpToLua(int node, int argnum, int parent_node) const
 
 		auto ship_entry = eval_ship(this_node);
 
-		if (!ship_entry || ship_entry->status != ShipStatus::PRESENT) {
+		if (!ship_entry || !ship_entry->shipp) {
 			// Name is invalid
 			return LuaValue::createValue(_action.getLuaState(), l_Ship.Set(object_h()));
 		}
@@ -284,7 +284,7 @@ luacpp::LuaValue LuaSEXP::sexpToLua(int node, int argnum, int parent_node) const
 		}
 
 		auto ship_entry = eval_ship(this_node);
-		if (!ship_entry || ship_entry->status != ShipStatus::PRESENT) {
+		if (!ship_entry || !ship_entry->shipp) {
 			// Name is invalid
 			return LuaValue::createValue(_action.getLuaState(), l_Ship.Set(object_h()));
 		}

code/playerman/playercontrol.cpp

@@ -1768,7 +1768,7 @@ void player_generate_death_message(player *player_p)
 
 	// killer_parent_name is always a ship name or a callsign (or blank).  If it's a ship name, get the ship and use the ship's display name
 	auto ship_entry = ship_registry_get(player_p->killer_parent_name);
-	auto killer_display_name = (ship_entry != nullptr && ship_entry->status == ShipStatus::PRESENT) ? ship_entry->shipp->get_display_name() : player_p->killer_parent_name;
+	auto killer_display_name = (ship_entry && ship_entry->shipp) ? ship_entry->shipp->get_display_name() : player_p->killer_parent_name;
 
 	switch (player_p->killer_objtype)
 	{
@@ -1785,7 +1785,7 @@ void player_generate_death_message(player *player_p)
 
 		case OBJ_WEAPON:
 			// is this from a friendly ship?
-			if ((ship_entry != nullptr) && (ship_entry->status == ShipStatus::PRESENT) && (Player_ship != nullptr) && (Player_ship->team == ship_entry->shipp->team))
+			if (ship_entry && ship_entry->shipp && Player_ship && (Player_ship->team == ship_entry->shipp->team))
 			{
 				sprintf(msg, XSTR( "%s was killed by friendly fire from %s", 1338), player_p->callsign, killer_display_name);
 			}
@@ -1827,7 +1827,7 @@ void player_generate_death_message(player *player_p)
 			else
 			{
 				// is this from a friendly ship?
-				if ((ship_entry != nullptr) && (ship_entry->status == ShipStatus::PRESENT) && (Player_ship != nullptr) && (Player_ship->team == ship_entry->shipp->team))
+				if (ship_entry && ship_entry->shipp && Player_ship && (Player_ship->team == ship_entry->shipp->team))
 				{
 					sprintf(msg, XSTR( "%s was destroyed by friendly beam fire from %s", 1339), player_p->callsign, killer_display_name);
 				}

code/ship/ship.h

@@ -906,10 +906,13 @@ enum class ShipStatus
 	// A ship is currently in-mission, and its objp and shipp pointers are valid
 	PRESENT,
 
-	// A ship is destroyed, departed, or vanished.  Note however that for destroyed ships,
-	// ship_cleanup is not called until the death roll is complete, which means there is a
-	// period of time where the ship is "exited", but objp and shipp are still valid and
-	// exited_index is not yet assigned.
+	// A ship has been destroyed but is still doing its death roll (not yet "exited");
+	// the objp and shipp pointers are still valid
+	DEATH_ROLL,
+
+	// A ship is destroyed, departed, or vanished; the objp and shipp pointers are nullptr.
+	// Note that for destroyed ships, ship_cleanup is not called until the death roll is complete,
+	// so the ship will spend a period of time in DEATH_ROLL before moving to EXITED.
 	EXITED
 };
 

code/ship/shiphit.cpp

@@ -1979,7 +1979,7 @@ void ship_hit_kill(object *ship_objp, object *other_obj, vec3d *hitpos, float pe
 
 	// Goober5000 - since we added a mission log entry above, immediately set the status.  For destruction, ship_cleanup isn't called until a little bit later
 	auto entry = &Ship_registry[Ship_registry_map[sp->ship_name]];
-	entry->status = ShipStatus::EXITED;
+	entry->status = ShipStatus::DEATH_ROLL;
 
 	ship_generic_kill_stuff( ship_objp, percent_killed );