Automated build commit

Fix 32-bit truncation in the flagset u64 round-trip

cmake/version.cmake

@@ -6,9 +6,9 @@ if (EXISTS "${PROJECT_SOURCE_DIR}/version_override.cmake")
     include("${PROJECT_SOURCE_DIR}/version_override.cmake")
 endif()
 
-set_if_not_defined(FSO_VERSION_MAJOR 25)
-set_if_not_defined(FSO_VERSION_MINOR 1)
-set_if_not_defined(FSO_VERSION_BUILD 0)
+set_if_not_defined(FSO_VERSION_MAJOR 26)
+set_if_not_defined(FSO_VERSION_MINOR 0)
+set_if_not_defined(FSO_VERSION_BUILD 1)
 
 # A numeric revision e.g. the CI build number
 set_if_not_defined(FSO_VERSION_REVISION 0)

code/ai/ai_flags.h

@@ -176,6 +176,10 @@ namespace AI {
 		Dont_limit_change_in_speed_due_to_physics_whack,
 		Guards_ignore_protected_attackers,
 		Fix_standard_strafe,
+		Fix_ai_target_recovery,	// a) strafing ships that lose their target re-process their orders, like chasing ships already do
+								// b) ships parked in AIM_NONE while holding a standing chase order (attack-any etc.) re-process their orders;
+								// c) target selection skips ships outside the attacker's actively-pursues list, and self-chosen targets
+								//    outside that list are dropped rather than held forever
 		Standard_strafe_used_more,
 		Unify_usage_ai_shield_manage_delay,
 		Fix_AI_shield_management_bug,
@@ -191,6 +195,11 @@ namespace AI {
 		Purge_player_issued_form_on_wing_after_subsequent_order,
 		Cancel_future_waves_of_any_wing_launched_from_an_exited_ship,
 		Fix_ignore_if_dead_flag,
+		Kamikaze_no_collision_avoidance,
+		Fix_big_ship_waypoint_completion,	// a) big ships complete a waypoint within their radius rather than sqrt(radius);
+											// b) completion no longer requires moving 0.1m in a single frame (framerate-dependent)
+		Fix_shockwave_expire_before_do_damage,	// shockwaves whose lifetime is shorter than one frame apply their area damage at least once before expiring
+		Fix_small_ai_recover_after_engines_repaired, // ensure small ship AI can switch back to useful AI modes if engines get repaired
 
 		NUM_VALUES
 	};

code/ai/ai_profiles.cpp

@@ -677,6 +677,8 @@ void parse_ai_profiles_tbl(const char *filename)
 
 				set_flag(profile, "$fix standard strafe:", AI::Profile_Flags::Fix_standard_strafe);
 
+				set_flag(profile, "$fix ai target recovery:", AI::Profile_Flags::Fix_ai_target_recovery);
+
 				set_flag(profile, "$standard strafe used more:", AI::Profile_Flags::Standard_strafe_used_more);
 
 				if (optional_string("$standard strafe triggers under this speed:")) {
@@ -691,6 +693,14 @@ void parse_ai_profiles_tbl(const char *filename)
 					stuff_float(&profile->strafe_max_unhit_time);
 				}
 
+				if (optional_string("$strafe retreat collide time:")) {
+					stuff_float(&profile->strafe_retreat_collide_time);
+				}
+
+				if (optional_string("$strafe retreat collide distance:")) {
+					stuff_float(&profile->strafe_retreat_collide_distance);
+				}
+
 				if (optional_string("$guard uses big-orbit for target radius above:")) {
 					stuff_float(&profile->guard_big_orbit_above_target_radius);
 				}
@@ -749,6 +759,13 @@ void parse_ai_profiles_tbl(const char *filename)
 
 				set_flag(profile, "$fix 'ignore if dead' flag:", AI::Profile_Flags::Fix_ignore_if_dead_flag);
 
+				set_flag(profile, "$no collision avoidance for kamikaze fighters:", AI::Profile_Flags::Kamikaze_no_collision_avoidance);
+
+				set_flag(profile, "$fix big ship waypoint completion:", AI::Profile_Flags::Fix_big_ship_waypoint_completion);
+
+				set_flag(profile, "$fix shockwave expiring before dealing damage:", AI::Profile_Flags::Fix_shockwave_expire_before_do_damage);
+
+				set_flag(profile, "$fix fighter/bomber AI recovers after engines repaired:", AI::Profile_Flags::Fix_small_ai_recover_after_engines_repaired);
 
 				// end of options ----------------------------------------
 
@@ -849,6 +866,8 @@ void ai_profile_t::reset()
 	standard_strafe_when_below_speed = 3.0f;
 	strafe_retreat_box_dist = 300.0f;
 	strafe_max_unhit_time = 20.0f;
+	strafe_retreat_collide_time = 2.0f;
+	strafe_retreat_collide_distance = 100.0f;
 
 	guard_big_orbit_above_target_radius = 500.0f;
 	guard_big_orbit_max_speed_percent = 1.0f;
@@ -975,4 +994,12 @@ void ai_profile_t::reset()
 	if (mod_supports_version(26, 0, 0)) {
 		flags.set(AI::Profile_Flags::Fix_ignore_if_dead_flag);
 	}
+	if (mod_supports_version(26, 2, 0)) {
+		flags.set(AI::Profile_Flags::Kamikaze_no_collision_avoidance);
+		flags.set(AI::Profile_Flags::Fix_big_ship_waypoint_completion);
+		flags.set(AI::Profile_Flags::Fix_shockwave_expire_before_do_damage);
+		flags.set(AI::Profile_Flags::Fix_small_ai_recover_after_engines_repaired);
+		flags.set(AI::Profile_Flags::Fix_standard_strafe);
+		flags.set(AI::Profile_Flags::Fix_ai_target_recovery);
+	}
 }

code/ai/ai_profiles.h

@@ -143,6 +143,8 @@ public:
 	float standard_strafe_when_below_speed; // Speed at which standard strafing large ships is possibly triggered
 	float strafe_retreat_box_dist;          // Distance beyond the bounding box to retreat to strafing point 
 	float strafe_max_unhit_time;            // Maximum amount of time to stay in strafe mode if not hit
+	float strafe_retreat_collide_time;		// When anticipated collision time is less than this, begin retreat from strafe
+	float strafe_retreat_collide_distance;	// When perpendicular distance to *surface* is less than this, begin retreat from strafe
 
 	// AI guard options  --wookieejedi
 	float guard_big_orbit_above_target_radius; // Radius of guardee that triggers ai_big_guard() 
@@ -161,7 +163,7 @@ extern int Num_ai_profiles;
 extern int Default_ai_profile;
 extern ai_profile_t Ai_profiles[MAX_AI_PROFILES];
 
-#define AI_PROFILES_INDEX(ai_p) ((int)((ai_p) - Ai_profiles))
+#define AI_PROFILES_INDEX(ai_p) (static_cast<int>((ai_p)-Ai_profiles))
 
 void ai_profiles_init();
 

code/ai/aibig.cpp

@@ -40,9 +40,6 @@
 
 // AI BIG MAGIC NUMBERS
 // Select strafing options are now exposed to modders  --wookieejedi
-#define	STRAFE_RETREAT_COLLIDE_TIME	2.0		// when anticipated collision time is less than this, begin retreat
-#define	STRAFE_RETREAT_COLLIDE_DIST	100		// when perpendicular distance to *surface* is less than this, begin retreat
-
 #define	EVADE_BOX_BASE_DISTANCE			300		// standard distance to end evade submode
 #define	EVADE_BOX_MIN_DISTANCE			200		// minimun distance to end evade submode, after long time
 
@@ -1198,70 +1195,78 @@ void ai_big_chase()
 		}
 
 		//	If a collision is expected, pull out!
-		float dist_normal_to_enemy;
+		if (!(The_mission.ai_profile->flags[AI::Profile_Flags::Kamikaze_no_collision_avoidance] && aip->ai_flags[AI::AI_Flags::Kamikaze])) {
 
-		if (vm_vec_mag_squared(&aip->big_attack_surface_normal) > 0.9) {
-			if (aip->ai_profile_flags[AI::Profile_Flags::Improved_subsystem_attack_pathing])
-				dist_normal_to_enemy = fl_abs((dist_to_enemy / vm_vec_dot(&vec_to_enemy, &aip->big_attack_surface_normal)));
-			else // I'm fairly sure this retail code VVV meant to use / instead of * 
-				dist_normal_to_enemy = fl_abs((dist_to_enemy * vm_vec_dot(&vec_to_enemy, &aip->big_attack_surface_normal)));
-		} else {
-			// don;t have normal so use a conservative value here
-			dist_normal_to_enemy = 0.3f * dist_to_enemy;
-		}
+			float dist_normal_to_enemy;
 
-//		float time_to_enemy = dist_normal_to_enemy / Pl_objp->phys_info.speed * fl_abs(vm_vec_dot(&Pl_objp->phys_info.vel, &aip->big_attack_surface_normal));
-//		if (Framecount % 30 == 1) {
-//			mprintf(("normal dist; %.1f, time: %.1f\n", dist_normal_to_enemy, time_to_enemy));
-//		}
-		
-		// since we're not in strafe and we may get a bad normal, cap dist_normal_to_enemy as MIN(0.3*dist_to_enemy, self)
-		// this will allow us to get closer on a bad normal
-		dist_normal_to_enemy = MAX(0.3f*dist_to_enemy, dist_normal_to_enemy);
-
-		if (dist_to_enemy < ATTACK_COLLIDE_BASE_DIST) {
-			// within 50m or 1sec
-			float time_to_enemy;
 			if (vm_vec_mag_squared(&aip->big_attack_surface_normal) > 0.9) {
-				if (Pl_objp->phys_info.speed > 0.1) {
-					time_to_enemy = dist_normal_to_enemy / fl_abs(vm_vec_dot(&Pl_objp->phys_info.vel, &aip->big_attack_surface_normal));
-				} else {
-					// a big time
-					time_to_enemy = 100.0f;
-				}
-			} else {
-				if (Pl_objp->phys_info.speed > 0.1) {
-					time_to_enemy = dist_normal_to_enemy / Pl_objp->phys_info.speed;
-				} else {
-					// a big time
-					time_to_enemy = 100.0f;
-				}
+				if (aip->ai_profile_flags[AI::Profile_Flags::Improved_subsystem_attack_pathing])
+					dist_normal_to_enemy = fl_abs((dist_to_enemy / vm_vec_dot(&vec_to_enemy, &aip->big_attack_surface_normal)));
+				else // I'm fairly sure this retail code VVV meant to use / instead of * 
+					dist_normal_to_enemy = fl_abs((dist_to_enemy * vm_vec_dot(&vec_to_enemy, &aip->big_attack_surface_normal)));
+			}
+			else {
+				// don;t have normal so use a conservative value here
+				dist_normal_to_enemy = 0.3f * dist_to_enemy;
 			}
 
-			float speed_dist = MAX(0.0f, (Pl_objp->phys_info.speed-50) * 2);
-			if ((dist_normal_to_enemy < ATTACK_COLLIDE_AVOID_DIST + speed_dist) || (time_to_enemy < ATTACK_COLLIDE_AVOID_TIME) ) {
-				// get away, simulate crsh recovery (don't use avoid)
-//				accelerate_ship(aip, -1.0f);
-				big_ship_collide_recover_start(Pl_objp, En_objp, nullptr);
-//				aip->submode = SM_AVOID;
-//				aip->submode_start_time = Missiontime;
-			} else if ((dist_normal_to_enemy < ATTACK_COLLIDE_SLOW_DIST) || (time_to_enemy < ATTACK_COLLIDE_SLOW_TIME) ) {
-				// slow down
-				accelerate_ship(aip, -1.0f);
-			}
-		}
+			//		float time_to_enemy = dist_normal_to_enemy / Pl_objp->phys_info.speed * fl_abs(vm_vec_dot(&Pl_objp->phys_info.vel, &aip->big_attack_surface_normal));
+			//		if (Framecount % 30 == 1) {
+			//			mprintf(("normal dist; %.1f, time: %.1f\n", dist_normal_to_enemy, time_to_enemy));
+			//		}
 
-		/*
-		if ((dot_to_enemy > 1.0f - 0.1f * En_objp->radius/(dist_to_enemy + 1.0f)) && (Pl_objp->phys_info.speed > dist_to_enemy/5.0f)) {
-			if (might_collide_with_ship(Pl_objp, En_objp, dot_to_enemy, dist_to_enemy, (aip->targeted_subsys == NULL)*2.0f + 1.5f)) {
-				if ((Missiontime - aip->last_hit_time > F1_0*4) && (dist_to_enemy < Pl_objp->radius*2 + En_objp->radius*2)) {
+					// since we're not in strafe and we may get a bad normal, cap dist_normal_to_enemy as MIN(0.3*dist_to_enemy, self)
+					// this will allow us to get closer on a bad normal
+			dist_normal_to_enemy = MAX(0.3f * dist_to_enemy, dist_normal_to_enemy);
+
+			if (dist_to_enemy < ATTACK_COLLIDE_BASE_DIST) {
+				// within 50m or 1sec
+				float time_to_enemy;
+				if (vm_vec_mag_squared(&aip->big_attack_surface_normal) > 0.9) {
+					if (Pl_objp->phys_info.speed > 0.1) {
+						time_to_enemy = dist_normal_to_enemy / fl_abs(vm_vec_dot(&Pl_objp->phys_info.vel, &aip->big_attack_surface_normal));
+					}
+					else {
+						// a big time
+						time_to_enemy = 100.0f;
+					}
+				}
+				else {
+					if (Pl_objp->phys_info.speed > 0.1) {
+						time_to_enemy = dist_normal_to_enemy / Pl_objp->phys_info.speed;
+					}
+					else {
+						// a big time
+						time_to_enemy = 100.0f;
+					}
+				}
+
+				float speed_dist = MAX(0.0f, (Pl_objp->phys_info.speed - 50) * 2);
+				if ((dist_normal_to_enemy < ATTACK_COLLIDE_AVOID_DIST + speed_dist) || (time_to_enemy < ATTACK_COLLIDE_AVOID_TIME)) {
+					// get away, simulate crsh recovery (don't use avoid)
+	//				accelerate_ship(aip, -1.0f);
+					big_ship_collide_recover_start(Pl_objp, En_objp, nullptr);
+					//				aip->submode = SM_AVOID;
+					//				aip->submode_start_time = Missiontime;
+				}
+				else if ((dist_normal_to_enemy < ATTACK_COLLIDE_SLOW_DIST) || (time_to_enemy < ATTACK_COLLIDE_SLOW_TIME)) {
+					// slow down
 					accelerate_ship(aip, -1.0f);
-				} else {
-					aip->submode = SM_AVOID;
-					aip->submode_start_time = Missiontime;
 				}
 			}
-		} */
+
+			/*
+			if ((dot_to_enemy > 1.0f - 0.1f * En_objp->radius/(dist_to_enemy + 1.0f)) && (Pl_objp->phys_info.speed > dist_to_enemy/5.0f)) {
+				if (might_collide_with_ship(Pl_objp, En_objp, dot_to_enemy, dist_to_enemy, (aip->targeted_subsys == NULL)*2.0f + 1.5f)) {
+					if ((Missiontime - aip->last_hit_time > F1_0*4) && (dist_to_enemy < Pl_objp->radius*2 + En_objp->radius*2)) {
+						accelerate_ship(aip, -1.0f);
+					} else {
+						aip->submode = SM_AVOID;
+						aip->submode_start_time = Missiontime;
+					}
+				}
+			} */
+		}
 	}
 
 	switch (aip->submode) {
@@ -1444,8 +1449,8 @@ static bool ai_big_strafe_maybe_retreat(const vec3d *target_pos)
 			collide_time = false;
 			collide_distance = false;
 		} else {
-			collide_time = (dist_to_target / Pl_objp->phys_info.speed) < STRAFE_RETREAT_COLLIDE_TIME;
-			collide_distance = dist_to_target < (STRAFE_RETREAT_COLLIDE_DIST + speed_to_dist_penalty);
+			collide_time = (dist_to_target / Pl_objp->phys_info.speed) < (The_mission.ai_profile->strafe_retreat_collide_time);
+			collide_distance = dist_to_target < ((The_mission.ai_profile->strafe_retreat_collide_distance) + speed_to_dist_penalty);
 		}
 	} else {
 		float dist_normal_to_target;
@@ -1455,8 +1460,8 @@ static bool ai_big_strafe_maybe_retreat(const vec3d *target_pos)
 		} else {
 			dist_normal_to_target = 0.2f * dist_to_target;
 		}
-		collide_time = (dist_normal_to_target / Pl_objp->phys_info.speed) < STRAFE_RETREAT_COLLIDE_TIME;
-		collide_distance = dist_normal_to_target < (STRAFE_RETREAT_COLLIDE_DIST + speed_to_dist_penalty);
+		collide_time = (dist_normal_to_target / Pl_objp->phys_info.speed) < (The_mission.ai_profile->strafe_retreat_collide_time);
+		collide_distance = dist_normal_to_target < ((The_mission.ai_profile->strafe_retreat_collide_distance) + speed_to_dist_penalty);
 	}
 
 	//if ((dot_to_enemy > 1.0f - 0.1f * En_objp->radius/(dist_to_enemy + 1.0f)) && (Pl_objp->phys_info.speed > dist_to_enemy/5.0f))

code/ai/aicode.cpp

@@ -2235,8 +2235,16 @@ int is_ignore_object(ai_info *aip, int objnum, int just_the_original = 0)
 	return 0;
 }
 
+//	Returns true if a ship of class type attacker_class_type is willing to chase a target of class type target_class_type.
+//	Mirrors the actively-pursues fallbacks in ai_chase(): if either class type is unknown, the chase is allowed.
+static bool ai_class_type_actively_pursues(int attacker_class_type, int target_class_type)
+{
+	if (attacker_class_type < 0 || target_class_type < 0)
+		return true;
 
-
+	const auto &pursues = Ship_types[attacker_class_type].ai_actively_pursues;
+	return std::any_of(pursues.begin(), pursues.end(), [target_class_type](int pursued_type) { return pursued_type == target_class_type; });
+}
 
 typedef struct eval_nearest_objnum {
 	int	objnum;
@@ -2244,6 +2252,7 @@ typedef struct eval_nearest_objnum {
 	int	enemy_team_mask;
 	int enemy_ship_info_index;
 	int enemy_class_type;
+	int	attacker_class_type;
 	int	enemy_wing;
 	float	range;
 	int	max_attackers;
@@ -2252,7 +2261,6 @@ typedef struct eval_nearest_objnum {
 	int	check_danger_weapon_objnum;
 } eval_nearest_objnum;
 
-
 void evaluate_object_as_nearest_objnum(eval_nearest_objnum *eno)
 {
 	ai_info	*aip;
@@ -2303,6 +2311,13 @@ void evaluate_object_as_nearest_objnum(eval_nearest_objnum *eno)
 				float	dist;
 				int	num_attacking;
 
+				// Don't pick a target that this ship's type refuses to chase, or it will park in AIM_NONE holding a target it never attacks.
+				// Only for unconstrained searches: an explicit chase-ship-class/type order should still produce a target even if ai_chase() doesn't chase it.
+				if (The_mission.ai_profile->flags[AI::Profile_Flags::Fix_ai_target_recovery]
+					&& eno->enemy_ship_info_index < 0 && eno->enemy_class_type < 0
+					&& !ai_class_type_actively_pursues(eno->attacker_class_type, Ship_info[shipp->ship_info_index].class_type))
+					return;
+
 				// Allow targeting of stealth in nebula by his firing at me
 				// This is done for a specific ship, not generally.
 				if ( !eno->check_danger_weapon_objnum ) {
@@ -2382,6 +2397,7 @@ int get_nearest_objnum(int objnum, int enemy_team_mask, int enemy_wing, float ra
 	eno.enemy_team_mask = enemy_team_mask;
 	eno.enemy_ship_info_index = ship_info_index;
 	eno.enemy_class_type = class_type;
+	eno.attacker_class_type = Ship_info[Ships[Objects[objnum].instance].ship_info_index].class_type;
 	eno.enemy_wing = enemy_wing;
 	eno.max_attackers = max_attackers;
 	eno.objnum = objnum;
@@ -2543,28 +2559,36 @@ int find_enemy(int objnum, float range, int max_attackers, int ship_info_index,
 
 	if (objnum < 0)
 		return -1;
+	object* objp = &Objects[objnum];
 
-	enemy_team_mask = iff_get_attackee_mask(obj_team(&Objects[objnum]));
+	enemy_team_mask = iff_get_attackee_mask(obj_team(objp));
 
 	//	if target_objnum != -1, use that as goal.
-	ai_info* aip = &Ai_info[Ships[Objects[objnum].instance].ai_index];
+	ship* shipp = &Ships[objp->instance];
+	ai_info* aip = &Ai_info[shipp->ai_index];
 	if (timestamp_elapsed(aip->choose_enemy_timestamp)) {
 		aip->choose_enemy_timestamp = timestamp(get_enemy_timestamp());
 
 		if (aip->target_objnum != -1) {
 			int target_objnum = aip->target_objnum;
+			object* target_objp = &Objects[target_objnum];
 
 			// DKA don't undo object as target in nebula missions.
 			// This could cause attack on ship on fringe on nebula to stop if attackee moves out of nebula range.  (BAD)
-			if (Objects[target_objnum].signature == aip->target_signature) {
-				ship* target_shipp = (Objects[target_objnum].type == OBJ_SHIP) ? &Ships[Objects[target_objnum].instance] : nullptr;
+			if (target_objp->signature == aip->target_signature) {
+				ship* target_shipp = (target_objp->type == OBJ_SHIP) ? &Ships[target_objp->instance] : nullptr;
 
 				if (target_shipp && iff_matches_mask(target_shipp->team, enemy_team_mask)) {
 					if (ship_info_index < 0 || ship_info_index == target_shipp->ship_info_index) {
-						if (class_type < 0 || (target_shipp->ship_info_index >= 0 &&
-							class_type == Ship_info[target_shipp->ship_info_index].class_type)) {
-							if (!(Objects[target_objnum].flags[Object::Object_Flags::Protected])) {
-								return target_objnum;
+						if (class_type < 0 || class_type == Ship_info[target_shipp->ship_info_index].class_type) {
+							if (!(target_objp->flags[Object::Object_Flags::Protected])) {
+								// same as get_nearest_objnum: only skip an unpursued target for unconstrained searches
+								if (!The_mission.ai_profile->flags[AI::Profile_Flags::Fix_ai_target_recovery]
+										|| ship_info_index >= 0 || class_type >= 0
+										|| ai_class_type_actively_pursues(Ship_info[shipp->ship_info_index].class_type,
+										                                  Ship_info[target_shipp->ship_info_index].class_type)) {
+									return target_objnum;
+								}
 							}
 						}
 					}
@@ -4936,14 +4960,24 @@ void ai_fly_to_target_position(const vec3d* target_pos, bool* pl_done_p=NULL, bo
 	}
 
 	float dist_to_cover_this_frame = (Pl_objp->phys_info.speed * flFrametime);
-	if ( (dist_to_goal < MIN_DIST_TO_WAYPOINT_GOAL) || dist_to_cover_this_frame > 0.1f ) {
+
+	// Retail's completion distance of sqrt(radius) is only ~32m for a capital ship with a 1000m radius --
+	// far smaller than such a ship's turning circle -- and retail also skipped the completion check
+	// entirely unless the ship covered 0.1m in a single frame, a framerate-dependent gate that excludes
+	// exactly the slow speeds at which a big ship could aim precisely.  Together these could trap a
+	// capital ship in an endless overshoot-turnaround-miss loop at its final waypoint.  With the flag
+	// set, big ships complete a waypoint anywhere within their own radius of it, and the check always runs.
+	bool fix_waypoint_completion = The_mission.ai_profile->flags[AI::Profile_Flags::Fix_big_ship_waypoint_completion];
+	float waypoint_tolerance = MIN_DIST_TO_WAYPOINT_GOAL + ((fix_waypoint_completion && sip->is_big_or_huge()) ? Pl_objp->radius : fl_sqrt(Pl_objp->radius));
+
+	if ( fix_waypoint_completion || (dist_to_goal < MIN_DIST_TO_WAYPOINT_GOAL) || dist_to_cover_this_frame > 0.1f ) {
 		vec3d	nearest_point;
 		float		r;
 
 		r = find_nearest_point_on_line(&nearest_point, &Pl_objp->last_pos, &Pl_objp->pos, target_pos);
 
-		if ( (dist_to_goal < (MIN_DIST_TO_WAYPOINT_GOAL + fl_sqrt(Pl_objp->radius) + dist_to_cover_this_frame))
-			|| (((r >= 0.0f) && (r <= 1.0f)) && (vm_vec_dist_quick(&nearest_point, target_pos) < (MIN_DIST_TO_WAYPOINT_GOAL + fl_sqrt(Pl_objp->radius)))))
+		if ( (dist_to_goal < (waypoint_tolerance + dist_to_cover_this_frame))
+			|| (((r >= 0.0f) && (r <= 1.0f)) && (vm_vec_dist_quick(&nearest_point, target_pos) < waypoint_tolerance)))
 		{
 				int treat_as_ship;
 
@@ -5672,6 +5706,14 @@ void ai_set_or_clear_preferred_primary_weapon(bool set_it, const object_ship_win
 	}
 }
 
+// find the first primary bank that actually contains a weapon, since banks might be empty for various reasons
+static int ai_first_loaded_primary_bank(const ship_weapon *swp)
+{
+	for (int i = 0; i < swp->num_primary_banks; i++)
+		if (swp->primary_bank_weapons[i] >= 0)
+			return i;
+	return -1;
+}
 
 //old version of this fuction, this will be useful for playing old missions and not having the new primary
 //selection code throw off the balance of the mission.
@@ -5689,7 +5731,7 @@ static int ai_select_primary_weapon_OLD(const object *objp, Weapon::Info_Flags f
 	Assert( shipp->ship_info_index >= 0 && shipp->ship_info_index < ship_info_size());
 
 	if (flags == Weapon::Info_Flags::Puncture) {
-		if (swp->current_primary_bank >= 0) {
+		if (swp->current_primary_bank >= 0 && swp->primary_bank_weapons[swp->current_primary_bank] >= 0) {
 			int	bank_index;
 
 			bank_index = swp->current_primary_bank;
@@ -5713,13 +5755,11 @@ static int ai_select_primary_weapon_OLD(const object *objp, Weapon::Info_Flags f
 		
 		// AL 26-3-98: If we couldn't find a puncture weapon, pick first available weapon if one isn't active
 		if ( swp->current_primary_bank < 0 ) {
-			if ( swp->num_primary_banks > 0 ) {
-				swp->current_primary_bank = 0;
-			}
+			swp->current_primary_bank = ai_first_loaded_primary_bank(swp);
 		}
 
 	} else {		//	Don't need to be using a puncture weapon.
-		if (swp->current_primary_bank >= 0) {
+		if (swp->current_primary_bank >= 0 && swp->primary_bank_weapons[swp->current_primary_bank] >= 0) {
 			if (!(Weapon_info[swp->primary_bank_weapons[swp->current_primary_bank]].wi_flags[Weapon::Info_Flags::Puncture])){
 				return swp->current_primary_bank;
 			}
@@ -5736,8 +5776,7 @@ static int ai_select_primary_weapon_OLD(const object *objp, Weapon::Info_Flags f
 		//	Wasn't able to find a non-puncture weapon.  Stick with what we have.
 	}
 
-	Assert( swp->current_primary_bank != -1 );		// get Alan or Allender
-
+	// this can legitimately be -1 if no bank contains a weapon
 	return swp->current_primary_bank;
 }
 
@@ -5770,6 +5809,11 @@ int ai_select_primary_weapon(object *objp, object *other_objp, Weapon::Info_Flag
 		return -1;
 	}
 
+	// if the ship has no primary banks, there is nothing to select
+	// (this can happen when e.g. a turreted gunship is ordered to disable another ship)
+	if (swp->num_primary_banks <= 0)
+		return -1;
+
 	bool other_is_ship = (other_objp->type == OBJ_SHIP);
 
 	if (other_is_ship)
@@ -5821,7 +5865,7 @@ int ai_select_primary_weapon(object *objp, object *other_objp, Weapon::Info_Flag
 	//made it so it only selects puncture weapons if the active goal is to disable something -Bobboau
 	if ((flags == Weapon::Info_Flags::Puncture) && ai_goal_is_disable_or_disarm(Ai_info[shipp->ai_index].goals[0].ai_mode))
 	{
-		if (swp->current_primary_bank >= 0) 
+		if (swp->current_primary_bank >= 0 && swp->primary_bank_weapons[swp->current_primary_bank] >= 0)
 		{
 			int	bank_index = swp->current_primary_bank;
 
@@ -5900,7 +5944,9 @@ int ai_select_primary_weapon(object *objp, object *other_objp, Weapon::Info_Flag
 		}
 		if (i_hullfactor_prev_bank == -1)		// In the unlikely instance we don't find at least 1 candidate weapon
 		{
-			i_hullfactor_prev_bank = 0;		// Just switch to the first one
+			i_hullfactor_prev_bank = ai_first_loaded_primary_bank(swp);		// Just switch to the first one with a weapon in it
+			if (i_hullfactor_prev_bank == -1)	// no bank contains a weapon
+				return -1;
 		}
 		swp->current_primary_bank = i_hullfactor_prev_bank;		// Select the best weapon
 		nprintf(("AI", "%i: Ship %s selecting weapon %s (no shields) vs target %s\n", Framecount, shipp->ship_name, Weapon_info[swp->primary_bank_weapons[i_hullfactor_prev_bank]].name, (other_is_ship ? other_shipp->ship_name : "non-ship") ));
@@ -5910,7 +5956,7 @@ int ai_select_primary_weapon(object *objp, object *other_objp, Weapon::Info_Flag
 	//if the shields are above lets say 10% definitely use a pierceing weapon if there are any-Bobboau
 	if (enemy_remaining_shield >= 0.10f)
 	{
-		if (swp->current_primary_bank >= 0) 
+		if (swp->current_primary_bank >= 0 && swp->primary_bank_weapons[swp->current_primary_bank] >= 0)
 		{
 			auto wip = &Weapon_info[swp->primary_bank_weapons[swp->current_primary_bank]];
 
@@ -5960,7 +6006,9 @@ int ai_select_primary_weapon(object *objp, object *other_objp, Weapon::Info_Flag
 		}
 		if (i_balancedfactor_prev_bank == -1)		// In the unlikely instance we don't find at least 1 candidate weapon
 		{
-			i_balancedfactor_prev_bank = 0;		// Just switch to the first one
+			i_balancedfactor_prev_bank = ai_first_loaded_primary_bank(swp);		// Just switch to the first one with a weapon in it
+			if (i_balancedfactor_prev_bank == -1)	// no bank contains a weapon
+				return -1;
 		}
 		swp->current_primary_bank = i_balancedfactor_prev_bank;		// Select the best weapon
 		nprintf(("AI", "%i: Ship %s selecting weapon %s (<50%% shields)\n", Framecount, shipp->ship_name, Weapon_info[swp->primary_bank_weapons[i_balancedfactor_prev_bank]].name));
@@ -5990,7 +6038,9 @@ int ai_select_primary_weapon(object *objp, object *other_objp, Weapon::Info_Flag
 		}
 		if (i_shieldfactor_prev_bank == -1)		// In the unlikely instance we don't find at least 1 candidate weapon
 		{
-			i_shieldfactor_prev_bank = 0;		// Just switch to the first one
+			i_shieldfactor_prev_bank = ai_first_loaded_primary_bank(swp);		// Just switch to the first one with a weapon in it
+			if (i_shieldfactor_prev_bank == -1)	// no bank contains a weapon
+				return -1;
 		}
 		swp->current_primary_bank = i_shieldfactor_prev_bank;		// Select the best weapon
 		nprintf(("AI", "%i: Ship %s selecting weapon %s (>50%% shields)\n", Framecount, shipp->ship_name, Weapon_info[swp->primary_bank_weapons[i_shieldfactor_prev_bank]].name));
@@ -6320,6 +6370,10 @@ int ai_fire_primary_weapon(object *objp)
 		aip->primary_select_timestamp = timestamp(5 * MILLISECONDS_PER_SECOND);	//	Maybe change primary weapon five seconds from now.
 	}
 
+	// if the ship has no primary weapon selected, whether because it has no primary banks or because no bank contains a weapon, then there is nothing to fire
+	if (swp->current_primary_bank < 0 || swp->primary_bank_weapons[swp->current_primary_bank] < 0)
+		return 0;
+
 	//We can only check LoS if we have a target defined
 	weapon_info* wip = &Weapon_info[swp->primary_bank_weapons[swp->current_primary_bank]];
 	if (aip->target_objnum != -1 && (The_mission.ai_profile->flags[AI::Profile_Flags::Require_exact_los] || wip->wi_flags[Weapon::Info_Flags::Require_exact_los])) {
@@ -7569,7 +7623,7 @@ static void get_behind_ship(ai_info *aip)
 
 	dot = vm_vec_dot(&vec_from_enemy, &En_objp->orient.vec.fvec);
 	
-	if (ai_willing_to_afterburn_hard(aip) && dot > 0.9f && vm_vec_dist(&Pl_objp->pos, &En_objp->pos) > 1000.0f) {
+	if (ai_willing_to_afterburn_hard(aip) && dot > 0.9f && vm_vec_dist_squared(&Pl_objp->pos, &En_objp->pos) > 1000.0f * 1000.0f) {
 		ai_afterburn_hard(Pl_objp, aip);
 	} else if (dot > 0.25f) {
 		accelerate_ship(aip, 1.0f);
@@ -7738,8 +7792,8 @@ void mabs_pick_goal_point(object *objp, object *big_objp, vec3d *collision_point
 				}
 			}
 
-			Assert(i != -1);
-			if (i != -1) {
+			Assert(min_index != -1);
+			if (min_index != -1) {
 				*avoid_pos = goals[min_index].pos;
 				return;
 			}
@@ -7813,7 +7867,7 @@ bool better_collision_avoidance_triggered(bool flag_to_check, float avoidance_ag
 		collide_vec *= radius_contribution;
 
 		collide_vec += pl_objp->pos;
-		return (maybe_avoid_big_ship(pl_objp, ignore_objp, &Ai_info[shipp->ai_index], &collide_vec, 0.f, 0.1f));
+		return (maybe_avoid_big_ship(pl_objp, ignore_objp, &Ai_info[shipp->ai_index], &collide_vec, avoidance_aggression, 0.1f));
 	}
 	return false;
 }
@@ -8991,15 +9045,8 @@ void ai_chase()
 		ship_info *esip = &Ship_info[Ships[En_objp->instance].ship_info_index];
 		if (esip->class_type > -1)
 		{
-			ship_type_info *stp = &Ship_types[sip->class_type];
-			size_t ap_size = stp->ai_actively_pursues.size();
-			for(size_t i = 0; i < ap_size; i++)
-			{
-				if(stp->ai_actively_pursues[i] == esip->class_type) {
-					go_after_it = true;
-					break;
-				}
-			}
+			const auto &pursues = Ship_types[sip->class_type].ai_actively_pursues;
+			go_after_it = std::any_of(pursues.begin(), pursues.end(), [esip](int pursued_type) { return pursued_type == esip->class_type; });
 		}
 		else
 		{
@@ -9012,6 +9059,15 @@ void ai_chase()
 
 	//WMC - Guess we do need this
 	if (!go_after_it) {
+		// If we picked this target ourselves -- through auto-attack, dynamic chase, or a standing chase order --
+		// drop it, so that the retargeting logic in ai_frame() can find something we are willing to chase.
+		if (The_mission.ai_profile->flags[AI::Profile_Flags::Fix_ai_target_recovery]) {
+			if (aip->active_goal < 0 || aip->active_goal == AI_ACTIVE_GOAL_DYNAMIC
+				|| (aip->active_goal < MAX_AI_GOALS && ai_goal_is_standing_chase(aip->goals[aip->active_goal].ai_mode))) {
+				aip->target_objnum = -1;
+				aip->target_signature = -1;
+			}
+		}
 		aip->mode = AIM_NONE;
 		return;
 	}
@@ -9137,6 +9193,9 @@ void ai_chase()
 						set_predicted_enemy_pos(&predicted_enemy_pos, Pl_objp, &aip->last_aim_enemy_pos, &aip->last_aim_enemy_vel, aip);
 				}
 			}
+		} else {
+			// no primary weapon to lead with, so just aim at the target
+			predicted_enemy_pos = enemy_pos;
 		}
 	}
 
@@ -12350,10 +12409,12 @@ void ai_process_subobjects(int objnum)
 
 				// handle ending animations
 				if ( (pss->turret_animation_position == MA_POS_READY) && timestamp_elapsed(pss->turret_animation_done_time) ) {
+					ship_info* turret_sip = &Ship_info[shipp->ship_info_index];
+					polymodel_instance* turret_pmi = model_get_instance(shipp->model_instance_num);
 					//For legacy animations using subtype for turret number
-					bool started = (Ship_info[shipp->ship_info_index].animations.getAll(model_get_instance(shipp->model_instance_num), animation::ModelAnimationTriggerType::TurretFiring, pss->system_info->subobj_num, true)
+					bool started = (turret_sip->animations.getAll(turret_pmi, animation::ModelAnimationTriggerType::TurretFiring, pss->system_info->subobj_num, true)
 						//For modern animations using proper triggered-by-subsys name
-						+ Ship_info[shipp->ship_info_index].animations.get(model_get_instance(shipp->model_instance_num), animation::ModelAnimationTriggerType::TurretFiring, animation::anim_name_from_subsys(pss->system_info)))
+						+ turret_sip->animations.get(turret_pmi, animation::ModelAnimationTriggerType::TurretFiring, animation::anim_name_from_subsys(pss->system_info)))
 						.start(animation::ModelAnimationDirection::RWD);
 					
 					if (started) {
@@ -12437,6 +12498,13 @@ void ai_process_subobjects(int objnum)
 				}
 			}
 		}
+	} 	
+	// Goober5000 and wookieejedi - conversely, if the engines are no longer blown (e.g. after repair), resume normal chase behavior.
+	// The rest of the AI assumes SM_ATTACK_FOREVER implies blown engines, 
+	// so don't leave the submode set after the engines recover, or the ship will never evade and will ignore being hit.
+	else if (The_mission.ai_profile->flags[AI::Profile_Flags::Fix_small_ai_recover_after_engines_repaired] && ((aip->mode == AIM_CHASE) && (aip->submode == SM_ATTACK_FOREVER))) {
+		aip->submode = SM_ATTACK;
+		aip->submode_start_time = Missiontime;
 	}
 }
 
@@ -13789,6 +13857,10 @@ void ai_manage_bay_doors(object *pl_objp, ai_info *aip, bool done)
 
 	ship *parent_ship = &Ships[shipp->bay_doors_parent_shipnum];
 
+	// the parent may have already been destroyed/departed, leaving stale bay door state behind
+	if (parent_ship->objnum < 0 || parent_ship->model_instance_num < 0)
+		return;
+
 	if (done)
 		parent_ship->bay_doors_wanting_open--;
 
@@ -15492,11 +15564,24 @@ void ai_frame(int objnum)
 		En_objp = NULL;
 	}
 
-	if (aip->mode == AIM_CHASE) {
-		if (En_objp == NULL) {
+	if (aip->mode == AIM_CHASE || ((The_mission.ai_profile->flags[AI::Profile_Flags::Fix_ai_target_recovery]) && aip->mode == AIM_STRAFE)) {
+		// If we're chasing or strafing against large ship but have lost our target, clear the active goal
+		// to ensure ai_process_mission_orders() re-evaluates our orders next frame.
+		// Without this fighter/bomber whose strafe target is destroyed drops to AIM_NONE (see ai_execute_behavior)
+		// with its active_goal still set, so a standing order like attack-any is never re-processed
+		// and the ship slows to zero and remains still until another order is issued.
+		if (En_objp == nullptr) {
 			aip->active_goal = -1;
 		}
 	}
+	// Similarly, a ship that has already dropped to AIM_NONE while holding a standing chase order can never
+	// recover on its own: ai_execute_behavior() does nothing in AIM_NONE, and ai_process_mission_orders()
+	// returns early while active_goal is set.  Clear the active goal so the order is re-processed next frame.
+	else if (The_mission.ai_profile->flags[AI::Profile_Flags::Fix_ai_target_recovery] && aip->mode == AIM_NONE
+		&& aip->active_goal >= 0 && aip->active_goal < MAX_AI_GOALS
+		&& ai_goal_is_standing_chase(aip->goals[aip->active_goal].ai_mode)) {
+		aip->active_goal = -1;
+	}
 
 	//	If there is a goal to resume and enough time has elapsed, resume the goal.
 	if ((aip->resume_goal_time > 0) && (aip->resume_goal_time < Missiontime)) {

code/ai/aigoals.h

@@ -106,6 +106,10 @@ inline bool ai_goal_is_specific_chase(ai_goal_mode ai_mode)
 {
 	return ai_mode == AI_GOAL_CHASE || ai_mode == AI_GOAL_CHASE_WING || ai_mode == AI_GOAL_CHASE_SHIP_CLASS || ai_mode == AI_GOAL_CHASE_SHIP_TYPE;
 }
+inline bool ai_goal_is_standing_chase(ai_goal_mode ai_mode)
+{
+	return ai_mode == AI_GOAL_CHASE_ANY;
+}
 
 enum class ai_achievability { ACHIEVABLE, NOT_ACHIEVABLE, NOT_KNOWN, SATISFIED };
 

code/ai/aiturret.cpp

@@ -1095,7 +1095,8 @@ int find_turret_enemy(const ship_subsys *turret_subsys, int objnum, const vec3d
 		int target_objnum = aip->target_objnum;
 
 		if (Objects[target_objnum].signature == aip->target_signature) {
-			ship* target_shipp = &Ships[Objects[target_objnum].instance];
+			// The parent's target can be a weapon or an asteroid, so make sure it's a ship before indexing Ships[]
+			ship* target_shipp = (Objects[target_objnum].type == OBJ_SHIP) ? &Ships[Objects[target_objnum].instance] : nullptr;
 			if (target_shipp && iff_matches_mask(target_shipp->team, enemy_team_mask)) {
 				if (!(Objects[target_objnum].flags[Object::Object_Flags::Protected])) { // check this flag as well
 					// nprintf(("AI", "Frame %i: Object %i resuming goal of object %i\n", AI_FrameCount, objnum,
@@ -1754,7 +1755,7 @@ bool turret_fire_weapon(int weapon_num,
 
 	parent_aip = &Ai_info[Ships[Objects[parent_objnum].instance].ai_index];
 	parent_ship = &Ships[Objects[parent_objnum].instance];
-	int turret_weapon_class = weapon_info_get_index(wip);
+	int turret_weapon_class = WEAPON_INFO_INDEX(wip);
 
 #ifndef NDEBUG
 	// moved here from check_ok_to_fire

code/bmpman/bmpman.cpp

@@ -83,7 +83,12 @@ static bool bm_inited = false;
 static uint Bm_next_signature = 0x1234;
 static int Bm_low_mem = 0;
 
-SCP_map<int,ubyte*> bm_lookup_cache;
+struct bm_lookup_cache_entry {
+	ubyte* data;
+	int width;
+	int height;
+};
+SCP_map<int, bm_lookup_cache_entry> bm_lookup_cache;
 
 /**
  * How much RAM bmpman can use for textures.
@@ -144,30 +149,31 @@ bitmap_slot* bm_get_slot(int handle, bool separate_ani_frames) {
 // Declaration of private functions and templates(declared as static type func(type param);)
 
 bitmap_lookup::bitmap_lookup(int bitmap_num):
-	Bitmap_data(NULL)
+	Bitmap_data(nullptr),
+	Width(0),
+	Height(0),
+	Num_channels(3)
 {
 	if ( !bm_is_valid(bitmap_num) ) return;
 
-	Num_channels = 3;
-
 	if ( bm_has_alpha_channel(bitmap_num) ) {
 		Num_channels = 4;
 	}
 
-	bitmap_entry *be = bm_get_entry(bitmap_num);
-	
-	Width = be->bm.w;
-	Height = be->bm.h;
-
-
 	auto cache_search = bm_lookup_cache.find(bitmap_num);
 	if (cache_search == bm_lookup_cache.end()) {
-		Bitmap_data = (ubyte*)vm_malloc(Width * Height * Num_channels * sizeof(ubyte));
+		// The texture in graphics memory may be smaller than the bitmap itself, e.g. when mipmap levels
+		// are culled at lower texture detail settings, so the readback reports the dimensions of the
+		// data it actually returns.
+		Bitmap_data = gr_get_bitmap_from_texture(bitmap_num, &Width, &Height);
 
-		gr_get_bitmap_from_texture((void*)Bitmap_data, bitmap_num);
-		bm_lookup_cache.insert({bitmap_num, Bitmap_data});
+		if (Bitmap_data != nullptr) {
+			bm_lookup_cache.insert({bitmap_num, {Bitmap_data, Width, Height}});
+		}
 	} else {
-		Bitmap_data = cache_search->second;
+		Bitmap_data = cache_search->second.data;
+		Width = cache_search->second.width;
+		Height = cache_search->second.height;
 	}
 }
 
@@ -190,6 +196,11 @@ float bitmap_lookup::get_channel_alpha(float u, float v)
 {
 	Assert( Bitmap_data != NULL );
 
+	// without an alpha channel there is nothing to look up, and indexing channel 3 would read out of bounds
+	if ( Num_channels < 4 ) {
+		return 1.0f;
+	}
+
 	int x = fl2i(map_texture_address(u) * (Width-1));
 	int y = fl2i(map_texture_address(v) * (Height-1));
 
@@ -198,7 +209,7 @@ float bitmap_lookup::get_channel_alpha(float u, float v)
 
 void clear_bm_lookup_cache() {
 	for(auto &iter: bm_lookup_cache) {
-		free(iter.second);
+		vm_free(iter.second.data);
 	}
 	bm_lookup_cache.clear();
 }

code/camera/camera.cpp

@@ -56,6 +56,10 @@ APPLY_TO_FOV_T(-, sub)
 
 // Used to set the default value for in-game options
 static float fov_default = DEFAULT_FOV;
+static float cockpit_fov_default = DEFAULT_FOV;
+static bool cockpit_fov_toggle_default = false;
+
+bool Use_cockpit_fov = false;
 
 static SCP_string fov_display(float val)
 {
@@ -73,6 +77,21 @@ static void parse_fov_func()
 	fov_default = value;
 }
 
+static void parse_cockpit_fov_func()
+{
+	float value;
+	stuff_float(&value);
+	CLAMP(value, 0.436332f, 1.5708f);
+	cockpit_fov_default = value;
+}
+
+static void parse_cockpit_fov_toggle_func()
+{
+	bool value;
+	stuff_boolean(&value);
+	cockpit_fov_toggle_default = value;
+}
+
 // coverity[GLOBAL_INIT_ORDER] -- safe; OptionBuilder::finish() uses Meyers singleton
 auto FovOption = options::OptionBuilder<float>("Graphics.FOV",
 					 std::pair<const char*, int>{"Field Of View", 1703},
@@ -90,25 +109,6 @@ auto FovOption = options::OptionBuilder<float>("Graphics.FOV",
 					 .parser(parse_fov_func)
 					 .finish();
 
-bool Use_cockpit_fov = false;
-
-// coverity[GLOBAL_INIT_ORDER] -- safe; OptionBuilder::finish() uses Meyers singleton
-auto CockpitFOVToggleOption = options::OptionBuilder<bool>("Graphics.CockpitFOVToggle",
-					 std::pair<const char*, int>{"Cockpit FOV Toggle", 1838},
-					 std::pair<const char*, int>{"Whether or not to use a different FOV for cockpit rendering from normal rendering", 1839})
-					 .category(std::make_pair("Graphics", 1825))
-					 .default_val(false)
-					 .change_listener([](bool val, bool) {
-					      if (!val) {
-					           COCKPIT_ZOOM_DEFAULT = VIEWER_ZOOM_DEFAULT;
-					      }
-					      return true; // This option will always persist so we never return false
-					 })
-					 .level(options::ExpertLevel::Advanced)
-					 .bind_to(&Use_cockpit_fov)
-					 .importance(61)
-					 .finish();
-
 // coverity[GLOBAL_INIT_ORDER] -- safe; OptionBuilder::finish() uses Meyers singleton
 auto CockpitFovOption = options::OptionBuilder<float>("Graphics.CockpitFOV",
 					 std::pair<const char*, int>{"Cockpit Field Of View", 1840},
@@ -124,9 +124,28 @@ auto CockpitFovOption = options::OptionBuilder<float>("Graphics.CockpitFOV",
 					      return true;
 					 })
 					 .display(fov_display)
-					 .default_val(fov_default)
+					 .default_func([]() { return cockpit_fov_default; })
 					 .level(options::ExpertLevel::Advanced)
 					 .importance(62)
+					 .parser(parse_cockpit_fov_func)
+					 .finish();
+
+// coverity[GLOBAL_INIT_ORDER] -- safe; OptionBuilder::finish() uses Meyers singleton
+auto CockpitFOVToggleOption = options::OptionBuilder<bool>("Graphics.CockpitFOVToggle",
+					 std::pair<const char*, int>{"Cockpit FOV Toggle", 1838},
+					 std::pair<const char*, int>{"Whether or not to use a different FOV for cockpit rendering from normal rendering", 1839})
+					 .category(std::make_pair("Graphics", 1825))
+					 .default_func([]() {return cockpit_fov_toggle_default;})
+					 .change_listener([](bool val, bool) {
+					      if (!val) {
+					           COCKPIT_ZOOM_DEFAULT = VIEWER_ZOOM_DEFAULT;
+					      }
+					      return true; // This option will always persist so we never return false
+					 })
+					 .level(options::ExpertLevel::Advanced)
+					 .bind_to(&Use_cockpit_fov)
+					 .importance(61)
+					 .parser(parse_cockpit_fov_toggle_func)
 					 .finish();
 
 //*************************CLASS: camera*************************

code/camera/photomode.cpp

@@ -249,7 +249,7 @@ void photo_mode_compute_hud_layout()
 		if (param.type == photo_mode_param_type::INT_RANGE) {
 			snprintf(buf, sizeof(buf), "%s: %d", XSTR(param.label, param.xstr_id), param.max_val);
 		} else {
-			snprintf(buf, sizeof(buf), "%s: %s", XSTR(param.label, param.xstr_id), XSTR("Off", 1285));
+			snprintf(buf, sizeof(buf), "%s: %s", XSTR(param.label, param.xstr_id), XSTR("Off", 1286));
 		}
 		measure(buf);
 	}

code/debris/debris.cpp

@@ -56,7 +56,7 @@ int Debris_num_submodels = 0;
 
 particle::ParticleEffectHandle Debris_hit_particle;
 
-#define	DEBRIS_INDEX(dp) (int)(dp-Debris.data())
+#define	DEBRIS_INDEX(dp) (static_cast<int>((dp)-Debris.data()))
 
 // Find the first available arc slot.  If none is available, and no_create is false, add one.
 debris_electrical_arc *debris_find_or_create_electrical_arc_slot(debris *db, bool no_create);

code/globalincs/flagset.h

@@ -194,9 +194,17 @@ class flagset {
 	bool any_set() const { return values.any(); }
 	bool none_set() const { return values.none(); }
 
-	void from_u64(std::uint64_t num) { values = (unsigned long) num; }
+	void from_u64(std::uint64_t num)
+	{
+		static_assert(SIZE <= 64, "from_u64 can only be used with flagsets of up to 64 flags");
+		values = std::bitset<SIZE>(num);
+	}
 
-	std::uint64_t to_u64() const { return (std::uint64_t) values.to_ulong(); }
+	std::uint64_t to_u64() const
+	{
+		static_assert(SIZE <= 64, "to_u64 can only be used with flagsets of up to 64 flags");
+		return static_cast<std::uint64_t>(values.to_ullong());
+	}
 
 	size_t hash() const { return std::hash<std::bitset<SIZE>>()(values); }
 };

code/graphics/2d.cpp

@@ -1432,6 +1432,27 @@ void gr_screen_resize(int width, int height)
 	gr_setup_viewport();
 }
 
+void gr_window_to_render_pos(float& x, float& y)
+{
+	if (!Cmdline_window_res) {
+		// Rendering goes straight to the window, so the two spaces are the same
+		return;
+	}
+
+	x *= i2fl(gr_screen.max_w) / static_cast<float>(Cmdline_window_res->first);
+	y *= i2fl(gr_screen.max_h) / static_cast<float>(Cmdline_window_res->second);
+}
+
+void gr_render_to_window_pos(float& x, float& y)
+{
+	if (!Cmdline_window_res) {
+		return;
+	}
+
+	x *= static_cast<float>(Cmdline_window_res->first) / i2fl(gr_screen.max_w);
+	y *= static_cast<float>(Cmdline_window_res->second) / i2fl(gr_screen.max_h);
+}
+
 int gr_get_resolution_class(int width, int height)
 {
 	if ((width >= GR_1024_THRESHOLD_WIDTH) && (height >= GR_1024_THRESHOLD_HEIGHT)) {

code/graphics/2d.h

@@ -831,7 +831,10 @@ typedef struct screen {
 	std::function<void()> gf_clear_states;
 
 	std::function<void(int bitmap_handle, int bpp, const ubyte* data, int width, int height)> gf_update_texture;
-	std::function<void(void* data_out, int bitmap_num)> gf_get_bitmap_from_texture;
+	// Reads the texture uploaded for a bitmap back from graphics memory.  Returns a vm_malloc'd buffer
+	// (freed by the caller with vm_free) and reports the dimensions of the returned data, which may be
+	// smaller than the bitmap if the texture was culled at upload; returns nullptr on failure.
+	std::function<ubyte*(int bitmap_num, int* width_out, int* height_out)> gf_get_bitmap_from_texture;
 
 	std::function<void(matrix4* shadow_view_matrix, const matrix* light_matrix, vec3d* eye_pos)> gf_shadow_map_start;
 	std::function<void()> gf_shadow_map_end;
@@ -1007,6 +1010,26 @@ extern bool gr_init(std::unique_ptr<os::GraphicsOperations>&& graphicsOps, int d
 extern void gr_screen_resize(int width, int height);
 extern int gr_get_resolution_class(int width, int height);
 
+/**
+ * @brief Converts a position or delta from OS window pixels into internal render-target pixels
+ *
+ * @details When -window_res is in effect the frame is rendered into an offscreen buffer sized
+ * gr_screen.max_w/max_h and only stretched to the window when we flip, so everything the engine
+ * draws lives in render space while SDL reports input in window space. These are pure scale
+ * factors with no offset, so they are equally valid for absolute positions and for deltas.
+ *
+ * Note that this is unrelated to the gr_resize_screen_pos() family, which handles the retail
+ * 640/1024 menu coordinate system.
+ */
+void gr_window_to_render_pos(float& x, float& y);
+
+/**
+ * @brief Converts a position or delta from internal render-target pixels into OS window pixels
+ *
+ * @see gr_window_to_render_pos
+ */
+void gr_render_to_window_pos(float& x, float& y);
+
 // Call this when your app ends.
 extern void gr_close();
 

code/graphics/grstub.cpp

@@ -259,9 +259,11 @@ void gr_stub_update_texture(int  /*bitmap_handle*/, int  /*bpp*/, const ubyte *
 {
 }
 
-void gr_stub_get_bitmap_from_texture(void*  /*data_out*/, int  /*bitmap_num*/)
+ubyte* gr_stub_get_bitmap_from_texture(int  /*bitmap_num*/, int* width_out, int* height_out)
 {
-
+	*width_out = 0;
+	*height_out = 0;
+	return nullptr;
 }
 
 int gr_stub_bm_make_render_target(int  /*n*/, int * /*width*/, int * /*height*/, int * /*bpp*/, int * /*mm_lvl*/, int  /*flags*/)

code/graphics/opengl/gropenglopenxr.cpp

@@ -63,7 +63,7 @@ bool gr_opengl_openxr_test_capabilities() {
 	}
 
 	if (requirements.minApiVersionSupported > XR_MAKE_VERSION(GLVersion.major, GLVersion.minor, 0)) {
-		mprintf(("System doesn't meet OpenXR graphics requirements (min %" PRIu64 ", available %" PRIu64 ")!\n", requirements.minApiVersionSupported, static_cast<XrVersion>(XR_MAKE_VERSION(GLVersion.major, GLVersion.minor, 0))));
+		mprintf(("System doesn't meet OpenXR graphics requirements (min " UINT64_T_ARG ", available " UINT64_T_ARG ")!\n", requirements.minApiVersionSupported, static_cast<XrVersion>(XR_MAKE_VERSION(GLVersion.major, GLVersion.minor, 0))));
 		return false;
 	}
 

code/graphics/opengl/gropengltexture.cpp

@@ -1413,15 +1413,20 @@ int opengl_get_texture( GLenum target, GLenum pixel_format, GLenum data_format,
 	return m_offset;
 }
 
-void gr_opengl_get_bitmap_from_texture(void* data_out, int bitmap_num)
+ubyte* gr_opengl_get_bitmap_from_texture(int bitmap_num, int* width_out, int* height_out)
 {
+	*width_out = 0;
+	*height_out = 0;
+
 	float u,v;
 
 	uint32_t array_index = 0;
-	gr_opengl_tcache_set(bitmap_num, TCACHE_TYPE_NORMAL, &u, &v, &array_index);
+	if ( !gr_opengl_tcache_set(bitmap_num, TCACHE_TYPE_NORMAL, &u, &v, &array_index) ) {
+		return nullptr;
+	}
 
 	auto *ts = bm_get_gr_info<tcache_slot_opengl>(bitmap_num, true);
-	
+
 	GLenum pixel_format = GL_RGB;
 	GLenum data_format = GL_UNSIGNED_BYTE;
 	int bytes_per_pixel = 3 * sizeof(ubyte);
@@ -1431,24 +1436,46 @@ void gr_opengl_get_bitmap_from_texture(void* data_out, int bitmap_num)
 		bytes_per_pixel = 4 * sizeof(ubyte);
 	}
 
-	// We can't read a specific layer of the texture so we need to read the entire texture and then memcpy the right part from that...
-	int num_frames = 0;
-	bm_get_info(bitmap_num, nullptr, nullptr, nullptr, &num_frames);
-	if (!bm_is_texture_array(bitmap_num)) {
-		num_frames = 1;
+	Assertion(ts->texture_target == GL_TEXTURE_2D_ARRAY, "Unexpected texture target encountered!");
+
+	// The texture in graphics memory may not match the dimensions or frame count that bmpman reports for
+	// this handle, e.g. when mipmap levels are culled at lower texture detail settings, so size the
+	// readback from what OpenGL will actually write, not from the bitmap or the texture cache slot.
+	GLint gl_width = 0, gl_height = 0, gl_layers = 0;
+	glGetTexLevelParameteriv(ts->texture_target, 0, GL_TEXTURE_WIDTH, &gl_width);
+	glGetTexLevelParameteriv(ts->texture_target, 0, GL_TEXTURE_HEIGHT, &gl_height);
+	glGetTexLevelParameteriv(ts->texture_target, 0, GL_TEXTURE_DEPTH, &gl_layers);
+
+	if ( (gl_width < 1) || (gl_height < 1) || (gl_layers < 1) || (array_index >= static_cast<uint32_t>(gl_layers)) ) {
+		mprintf(("Cannot read bitmap %d (%s) back from its texture; OpenGL reports level 0 as %dx%d with %d layer(s) but layer %u is needed.\n",
+			bitmap_num, bm_get_filename(bitmap_num), gl_width, gl_height, gl_layers, array_index));
+		return nullptr;
 	}
 
-	// The size of a single frame in the array
-	auto slice_size = ts->w * ts->h * bytes_per_pixel;
-	std::unique_ptr<std::uint8_t[]> buffer(new std::uint8_t[num_frames * slice_size]);
+	// At maximum texture detail no culling occurs on upload, so the texture must match the dimensions
+	// recorded in the texture cache slot; if this trips, we have another slot mismatch.
+	Assertion((Detail.hardware_textures < 4) || ((gl_width == ts->w) && (gl_height == ts->h)),
+		"Texture readback size mismatch for bitmap %d (%s): OpenGL reports level 0 as %dx%d but the texture cache slot expects %dx%d.",
+		bitmap_num, bm_get_filename(bitmap_num), gl_width, gl_height, ts->w, ts->h);
 
-	Assertion(ts->texture_target == GL_TEXTURE_2D_ARRAY, "Unexpected texture target encountered!");
+	// The size of a single layer in the array
+	size_t slice_size = static_cast<size_t>(gl_width) * gl_height * bytes_per_pixel;
+
+	// We can't read a specific layer of the texture so we need to read the entire texture and then memcpy the right part from that...
+	// Some drivers write tiny compressed textures back in whole 4x4 block granularity, so pad each layer
+	// of the staging buffer to full blocks to keep any such overwrite within bounds.
+	size_t padded_slice_size = static_cast<size_t>((gl_width + 3) & ~3) * ((gl_height + 3) & ~3) * bytes_per_pixel;
+	std::unique_ptr<std::uint8_t[]> buffer(new std::uint8_t[padded_slice_size * gl_layers]);
 
 	// Copy the entire texture level into the bitmap
 	glGetTexImage(ts->texture_target, 0, pixel_format, data_format, buffer.get());
 
-	auto buffer_offset = array_index * slice_size;
-	memcpy(data_out, buffer.get() + buffer_offset, slice_size);
+	auto data_out = reinterpret_cast<ubyte*>(vm_malloc(slice_size));
+	memcpy(data_out, buffer.get() + array_index * slice_size, slice_size);
+
+	*width_out = gl_width;
+	*height_out = gl_height;
+	return data_out;
 }
 
 void gr_opengl_get_texture_scale(int bitmap_handle, float *u_scale, float *v_scale)
@@ -1494,7 +1521,7 @@ size_t opengl_export_render_target( int slot, int width, int height, int alpha,
 		return 0;
 	}
 
-	if ( (ts->w != width) && (ts->h != height) ) {
+	if ( (ts->w != width) || (ts->h != height) ) {
 		mprintf(("OpenGL ERROR: Passed width and height do not match values for texture!\n"));
 		return 0;
 	}

code/graphics/opengl/gropengltexture.h

@@ -83,7 +83,7 @@ void opengl_preload_init();
 void opengl_kill_render_target(bitmap_slot* slot);
 int opengl_make_render_target(int handle, int *w, int *h, int *bpp, int *mm_lvl, int flags);
 int opengl_set_render_target(int slot, int face = -1, int is_static = 0);
-void gr_opengl_get_bitmap_from_texture(void* data_out, int bitmap_num);
+ubyte* gr_opengl_get_bitmap_from_texture(int bitmap_num, int* width_out, int* height_out);
 size_t opengl_export_render_target( int slot, int width, int height, int alpha, int num_mipmaps, ubyte *image_data );
 void opengl_set_texture_target(GLenum target = GL_TEXTURE_2D);
 void opengl_set_texture_face(GLenum face = GL_TEXTURE_2D);

code/graphics/vulkan/vulkan_stubs.cpp

@@ -106,7 +106,12 @@ void stub_clear_states() {}
 
 void stub_update_texture(int /*bitmap_handle*/, int /*bpp*/, const ubyte* /*data*/, int /*width*/, int /*height*/) {}
 
-void stub_get_bitmap_from_texture(void* /*data_out*/, int /*bitmap_num*/) {}
+ubyte* stub_get_bitmap_from_texture(int /*bitmap_num*/, int* width_out, int* height_out)
+{
+	*width_out = 0;
+	*height_out = 0;
+	return nullptr;
+}
 
 int stub_bm_make_render_target(int /*n*/, int* /*width*/, int* /*height*/, int* /*bpp*/, int* /*mm_lvl*/, int /*flags*/)
 {

code/hud/hudsquadmsg.cpp

@@ -104,8 +104,8 @@ int Menu_pgdn_coords[GR_NUM_RESOLUTIONS][2] = {
 // following defines/vars are used to build menus that are used in messaging mode
 
 char Squad_msg_title[256] = "";
-mmode_item MsgItems[MAX_MENU_ITEMS];
-int Num_menu_items = -1; // number of items for a message menu
+SCP_vector<mmode_item> MsgItems;
+static bool Rebuild_MsgItems = true;
 
 static int First_menu_item = -1;							// index of first item in the menu. This tracks what element of comms options collection is displayed as first option, and displays the next 9 options. Changes only by +/- MAX_MENU_ITEMS (10)
 static int Selected_menu_item = First_menu_item;           //!< index of selected item in the menu. Possible index range: 0 - 9, assuming MAX_MENU_ITEMS == 10, and First_menu_item gets initialized
@@ -218,10 +218,11 @@ void hud_squadmsg_start()
 	Msg_key = -1;
 	Msg_key_set_from_scripting = false;
 
-	Num_menu_items = -1;													// reset the menu items
+	Rebuild_MsgItems = true;												// reset the menu items
+	MsgItems.clear();
 	First_menu_item = 0;
 	Selected_menu_item = First_menu_item;                            // make first menu item a selected object
-	Display_selector = false;
+	Display_selector = Always_show_selected_item_in_comms_gauge;
 
 	Squad_msg_mode = SM_MODE_TYPE_SELECT;							// start off at the base state
 	Msg_mode_timestamp = _timestamp(DEFAULT_MSG_TIMEOUT);		// initialize our timer to bogus value
@@ -376,11 +377,7 @@ int hud_squadmsg_count_ships(int add_to_menu)
 		count++;
 		if (add_to_menu)
 		{
-			Assert ( Num_menu_items < MAX_MENU_ITEMS );
-			MsgItems[Num_menu_items].text = shipp->get_display_name();
-			MsgItems[Num_menu_items].instance = SHIP_INDEX(shipp);
-			MsgItems[Num_menu_items].active = 1;
-			Num_menu_items++;
+			MsgItems.push_back({SHIP_INDEX(shipp), 1, shipp->get_display_name()});
 		}
 	}
 
@@ -431,11 +428,7 @@ int hud_squadmsg_count_wings( int add_to_menu )
 		if ( hud_squadmsg_wing_valid(&Wings[wingnum]) ) {
 			count++;
 			if ( add_to_menu ) {
-				Assert ( Num_menu_items < MAX_MENU_ITEMS );
-				MsgItems[Num_menu_items].text = Wings[wingnum].get_display_name();
-				MsgItems[Num_menu_items].instance = wingnum;
-				MsgItems[Num_menu_items].active = 1;
-				Num_menu_items++;
+				MsgItems.push_back({wingnum, 1, Wings[wingnum].get_display_name()});
 			}
 		}
 	}
@@ -452,11 +445,7 @@ int hud_squadmsg_count_wings( int add_to_menu )
 		if ( hud_squadmsg_wing_valid(&Wings[i]) ) {
 			count++;
 			if ( add_to_menu ) {
-				Assert ( Num_menu_items < MAX_MENU_ITEMS );
-				MsgItems[Num_menu_items].text = Wings[i].get_display_name();
-				MsgItems[Num_menu_items].instance = i;
-				MsgItems[Num_menu_items].active = 1;
-				Num_menu_items++;
+				MsgItems.push_back({i, 1, Wings[i].get_display_name()});
 			}
 		}
 	}
@@ -469,15 +458,17 @@ int hud_squadmsg_count_wings( int add_to_menu )
 void hud_squadmsg_do_mode( int mode )
 {
 	Squad_msg_mode = mode;
-	Num_menu_items = -1;
+	MsgItems.clear();
+	Rebuild_MsgItems = true;
 	First_menu_item = 0;
+	Selected_menu_item = 0;   // keep offset consistent with the reset page across submenu transitions
 }
 
 void hud_squadmsg_page_down()
 {
-	if ( (First_menu_item + MAX_MENU_DISPLAY) < Num_menu_items ) {
+	if ( (First_menu_item + MAX_MENU_DISPLAY) < sz2i(MsgItems.size()) ) {
 		First_menu_item += MAX_MENU_DISPLAY;
-		Assert ( First_menu_item < Num_menu_items );
+		Assert ( First_menu_item < sz2i(MsgItems.size()) );
 	}
 }
 
@@ -490,77 +481,52 @@ void hud_squadmsg_page_up()
 }
 
 //Fuctions that allow selection of specific comms menu items with simple up/down/select buttons
-void hud_squadmsg_selection_move_down() {
+void hud_squadmsg_selection_move( bool up ) {
+	if (Rebuild_MsgItems || MsgItems.empty())
+		return;
 
 	//Check if comms menu is up
 	if (Player->flags & PLAYER_FLAGS_MSG_MODE)
 	{
-		//move down
-		++Selected_menu_item;
 		Display_selector = true;
 
 		//play scrolling sound and reset the comms window timeout timer, so the window doesn't disappear while we select our item
 		gamesnd_play_iface(InterfaceSounds::SCROLL);
 		Msg_mode_timestamp = _timestamp(DEFAULT_MSG_TIMEOUT);
 
-		//Move to next page if we went outside of current one
-		if (Selected_menu_item == MAX_MENU_DISPLAY 
-			&& (First_menu_item + MAX_MENU_DISPLAY < Num_menu_items))
-		{
-			hud_squadmsg_page_down();
-			Selected_menu_item = 0;
+		//The selection moves +/-1 through the whole menu, wrapping around at either end.
+		//First_menu_item and Selected_menu_item are just the page and the offset within the page.
+		int num_items = sz2i(MsgItems.size());
+		int step = up ? -1 : 1;
+		int selected_index = First_menu_item + Selected_menu_item;
+
+		//skip over any hidden items (such as when using Hide_main_rearm_items_in_comms_gauge).
+		//bound the search by the item count so a fully hidden menu does not loop forever.
+		for (int i = 0; i < num_items; i++) {
+			selected_index = (selected_index + step + num_items) % num_items;
+
+			//stop once we land on a visible item
+			if (MsgItems[selected_index].active >= 0)
+				break;
 		}
 
-		//Select the first menu item if we went outside items range, so we can loop around
-		if (First_menu_item + Selected_menu_item >= Num_menu_items) 
-		{
-			First_menu_item = 0;
-			Selected_menu_item = First_menu_item;
-		}
-	}
-}
-
-void hud_squadmsg_selection_move_up() {
-
-	//Check if comms menu is up
-	if (Player->flags & PLAYER_FLAGS_MSG_MODE)
-	{
-		//move up
-		--Selected_menu_item;
-		Display_selector = true;
-
-		//play scrolling sound and reset the comms window timeout timer, so the window doesn't disappear while we select our item
-		gamesnd_play_iface(InterfaceSounds::SCROLL);
-		Msg_mode_timestamp = _timestamp(DEFAULT_MSG_TIMEOUT);
-
-		//Move to previous page if it exists
-		if (Selected_menu_item < 0 && First_menu_item > 0)
-		{
-			hud_squadmsg_page_up();
-			Selected_menu_item = MAX_MENU_DISPLAY - 1; //if we're moving to previous page in the first place, we assume it was already populated to the max
-		}
-
-		//Select the last menu item if we went outside items range, so we can loop around
-		else if (Selected_menu_item < 0) 
-		{
-			//Assuming MAX_MENU_DISPLAY = 10, set First_menu_item to the nearest lower multiple of 10
-			//So if we have 85 items in comms menu, looping back from 1st page to last would set First_menu_item to 80
-			//exactly like pageUp/pageDown does
-			First_menu_item = ((Num_menu_items - 1) / MAX_MENU_DISPLAY) * MAX_MENU_DISPLAY;
-			Selected_menu_item = Num_menu_items - 1 - First_menu_item;
-		}
+		Selected_menu_item = selected_index % MAX_MENU_DISPLAY;
+		First_menu_item = selected_index - Selected_menu_item;
 	}
 }
 
 //function that tricks hud_squadmsg_get_key() into thinking player selected a menu item with a num key press
 //Yes, this is a pretty much a hack, but it's simple and works with every squadmsg type.
 void hud_squadmsg_selection_select() {
+	if (Rebuild_MsgItems) {
+		return;
+	}
 	
 	//Check if comms menu is up
 	if (Player->flags & PLAYER_FLAGS_MSG_MODE)
 	{
 		//Check if selected option is even active
-		if ((MsgItems[Selected_menu_item + First_menu_item].active > 0))
+		if ((sz2i(MsgItems.size()) > Selected_menu_item + First_menu_item) && (MsgItems[Selected_menu_item + First_menu_item].active > 0))
 		{
 			Msg_key_used = 1;
 			Msg_key = Selected_menu_item + 2;	  //+1 because menu items on actual menu start from 1, not 0
@@ -579,7 +545,7 @@ int hud_squadmsg_get_total_keys()
 	int num_keys_used;
 
 	num_keys_used = MAX_KEYS_NO_SCROLL;
-	if ( Num_menu_items > MAX_MENU_DISPLAY )
+	if ( sz2i(MsgItems.size()) > MAX_MENU_DISPLAY )
 		num_keys_used = MAX_KEYS_USED;
 
 	return num_keys_used;
@@ -669,15 +635,16 @@ int hud_squadmsg_get_key()
 			} else if ( k == KEY_ESC ) {
 				hud_squadmsg_toggle();
 				return -1;
-			} else if ( (i < Num_menu_items) && (Squad_msg_mode == SM_MODE_REINFORCEMENTS) )		// return any key if selecting reinforcement
+			} else if ( (i < sz2i(MsgItems.size())) && (Squad_msg_mode == SM_MODE_REINFORCEMENTS) ) {		// return any key if selecting reinforcement
 				return i;
+			}
 
 			// play general fail sound if inactive item hit.
-			else if ((i + First_menu_item < Num_menu_items) && (MsgItems[i + First_menu_item].active == 0)) {
+			else if ((i + First_menu_item < sz2i(MsgItems.size())) && (MsgItems[i + First_menu_item].active == 0)) {
 				gamesnd_play_iface(InterfaceSounds::GENERAL_FAIL);
 			}
 
-			else if ((i + First_menu_item < Num_menu_items) && (MsgItems[i + First_menu_item].active > 0)) {	// only return keys that are associated with menu items
+			else if ((i + First_menu_item < sz2i(MsgItems.size())) && (MsgItems[i + First_menu_item].active > 0)) {	// only return keys that are associated with menu items
 				return i + First_menu_item;
 			}
 
@@ -1779,24 +1746,26 @@ void hud_squadmsg_type_select( )
 
 	num_order_types += (int)lua_cat_list.size();
 
+	MsgItems.clear();
+	Rebuild_MsgItems = false;
+
 	// Add the items
 	for (i = 0; i < num_order_types; i++)
 	{
+
 		if (i < NUM_COMM_ORDER_TYPES) {
-			MsgItems[i].text = Comm_order_types[i];
+			MsgItems.push_back({0, 1, Comm_order_types[i]}); // assume active
 		} else {
-			MsgItems[i].text = lua_cat_list[i - NUM_COMM_ORDER_TYPES];
+			MsgItems.push_back({0, 1, lua_cat_list[i - NUM_COMM_ORDER_TYPES]}); // assume active
 		}
-		MsgItems[i].active = 1;						// assume active
 	}
-	Num_menu_items = num_order_types;
 
 
 	// check to see if the player is a traitor.  If so, then he will not
 	// be able to do anything from this menu
 	if ((Player_ship != NULL) && (Player_ship->team == Iff_traitor)) {
-		for (i = 0; i < MAX_MENU_ITEMS; i++ )
-			MsgItems[i].active = 0;
+		for (auto &item : MsgItems)
+			item.active = 0;
 		goto do_main_menu;
 	}
 
@@ -1833,8 +1802,8 @@ void hud_squadmsg_type_select( )
 	//
 	// also, only allow support ship if this player is not allowed to messaage. 
 	if ( (hud_communications_state(Player_ship) != COMM_OK) || ((Game_mode & GM_MULTIPLAYER) && !multi_can_message(Net_player)) ) {
-		for ( i = 0; i < MAX_MENU_ITEMS; i++ ){
-			MsgItems[i].active = 0;
+		for (auto &item : MsgItems){
+			item.active = 0;
 		}
 
 		MsgItems[TYPE_REPAIR_REARM_ITEM].active = Hide_main_rearm_items_in_comms_gauge ? -1 : 1;
@@ -1877,7 +1846,7 @@ do_main_menu:
 	strcpy_s(Squad_msg_title, XSTR( "Message What", 316));
 	k = hud_squadmsg_get_key();
 	if ( k != -1 ) {							// when k != -1, we have a key that associates with menu item
-		Assert ( k < Num_menu_items );
+		Assert ( k < sz2i(MsgItems.size()) );
 		if ( k == TYPE_SHIP_ITEM ){
 			hud_squadmsg_do_mode( SM_MODE_SHIP_SELECT );
 		} else if ( k == TYPE_WING_ITEM ) {
@@ -1907,8 +1876,9 @@ void hud_squadmsg_ship_select()
 {
 	int k;
 
-	if ( Num_menu_items == -1 ) {
-		Num_menu_items = 0;
+	if ( Rebuild_MsgItems ) {
+		MsgItems.clear();
+		Rebuild_MsgItems = false;
 		hud_squadmsg_count_ships( 1 );
 	}
 
@@ -1934,8 +1904,9 @@ void hud_squadmsg_wing_select()
 {
 	int k;
 
-	if ( Num_menu_items == -1 ) {
-		Num_menu_items = 0;
+	if ( Rebuild_MsgItems ) {
+		MsgItems.clear();
+		Rebuild_MsgItems = false;
 		hud_squadmsg_count_wings( 1 );
 	}
 
@@ -2045,8 +2016,9 @@ void hud_squadmsg_call_reinforcement(reinforcements &reinforcement, int  /*playe
 // function to display a list of reinforcements available to the player
 void hud_squadmsg_reinforcement_select()
 {
-	if ( Num_menu_items == -1 ) {
-		Num_menu_items = 0;
+	if ( Rebuild_MsgItems ) {
+		MsgItems.clear();
+		Rebuild_MsgItems = false;
 
 		int i = -1;
 		for (const auto &reinforcement: Reinforcements) {
@@ -2092,16 +2064,7 @@ void hud_squadmsg_reinforcement_select()
 				r_name = p_objp->get_display_name();	// this will handle getting rid of the hash if necessary
 			}
 
-			Assert ( Num_menu_items < MAX_MENU_ITEMS );
-			MsgItems[Num_menu_items].text = r_name;
-			MsgItems[Num_menu_items].instance = i;
-			MsgItems[Num_menu_items].active = 0;
-
-			if ( reinforcement.flags & RF_IS_AVAILABLE ) {
-				MsgItems[Num_menu_items].active = 1;
-			}
-
-			Num_menu_items++;
+			MsgItems.push_back({i, (reinforcement.flags & RF_IS_AVAILABLE) ? 1 : 0, r_name}); // set the item to active if reinforcement is available
 		}
 	}
 
@@ -2158,25 +2121,23 @@ void hud_squadmsg_ship_command()
 		default_orders.insert(default_messages.cbegin(), default_messages.cend());
 	}
 
-	Num_menu_items = 0;
+	MsgItems.clear();
+	Rebuild_MsgItems = false;
 	for(size_t order_id : default_orders) {
-		Assert (Num_menu_items < MAX_MENU_ITEMS);
-		MsgItems[Num_menu_items].text = Player_orders[order_id].localized_name;
-		MsgItems[Num_menu_items].instance = (int)order_id;
-		MsgItems[Num_menu_items].active = 0;
+		MsgItems.push_back({(int)order_id, 0, Player_orders[order_id].localized_name});
 
 		// check the bit to see if the command is active
 		if (orders.find(order_id) != orders.end())
-			MsgItems[Num_menu_items].active = 1;
+			MsgItems.back().active = 1;
 
 		// if the order cannot be carried out by the ship, then item should be inactive
 		if ((Msg_instance != MESSAGE_ALL_FIGHTERS) && !hud_squadmsg_ship_order_valid(Msg_instance, (int)order_id))
-			MsgItems[Num_menu_items].active = 0;
+			MsgItems.back().active = 0;
 
 		// do some other checks to possibly gray out other items.
 		// if no target, remove any items which are associated with the players target
 		if (!hud_squadmsg_is_target_order_valid(order_id, nullptr))
-			MsgItems[Num_menu_items].active = 0;
+			MsgItems.back().active = 0;
 
 		// if messaging all fighters, see if we should gray out the order if no one will accept it,
 		// or modify the text if only some of the ships will accept it
@@ -2208,14 +2169,13 @@ void hud_squadmsg_ship_command()
 			if (!all_accept) {
 				// either modify the text if a partial accept, or grey it out if no one accepts
 				if (partial_accept) {
-					MsgItems[Num_menu_items].text += XSTR("(*)", 320);
+					MsgItems.back().text += XSTR("(*)", 320);
 				} else {
-					MsgItems[Num_menu_items].active = 0;
+					MsgItems.back().active = 0;
 				}
 			}
 		}
 
-		Num_menu_items++;
 	
 	}
 
@@ -2225,7 +2185,7 @@ void hud_squadmsg_ship_command()
 	// when we get a valid goal, we must add the goal to the ai ship's goal list
 
 	if ( k != -1 ) {
-		Assert ( k < Num_menu_items );
+		Assert ( k < sz2i(MsgItems.size()) );
 		// when messaging all fighters or ignoring target, call the send_to_all_fighters routine
 		// Goober5000 - ignore no longer sends to all fighters
 		if (Msg_instance == MESSAGE_ALL_FIGHTERS)
@@ -2241,9 +2201,9 @@ void hud_squadmsg_msg_general()
 {
 	int k;
 
-	Num_menu_items = 0;
+	MsgItems.clear();
+	Rebuild_MsgItems = false;
 	for (size_t order_id = 0; order_id < Player_orders.size(); order_id++) {
-		Assert(Num_menu_items < MAX_MENU_ITEMS);
 
 		if (Player_orders[order_id].lua_id <= 0) {
 			continue;
@@ -2263,17 +2223,12 @@ void hud_squadmsg_msg_general()
 
 		//Only add it if it is enabled for the mission
 		if (lua_porder->cur_enabled) {
-
-			MsgItems[Num_menu_items].text = Player_orders[order_id].localized_name;
-			MsgItems[Num_menu_items].instance = Player_orders[order_id].lua_id;
-			MsgItems[Num_menu_items].active = (int)lua_porder->cur_valid;
+			MsgItems.push_back({Player_orders[order_id].lua_id, (int)lua_porder->cur_valid, Player_orders[order_id].localized_name});
 
 			// do some other checks to possibly gray out other items.
 			// if no target, remove any items which are associated with the players target
 			if (!hud_squadmsg_is_target_order_valid(order_id, nullptr))
-				MsgItems[Num_menu_items].active = 0;
-
-			Num_menu_items++;
+				MsgItems.back().active = 0;
 		}
 	}
 
@@ -2283,7 +2238,7 @@ void hud_squadmsg_msg_general()
 	// when we get a valid goal, we must add the goal to the ai ship's goal list
 
 	if (k != -1) {
-		Assert(k < Num_menu_items);
+		Assert( k < sz2i(MsgItems.size()) );
 		
 		ai_lua_start_general(MsgItems[k].instance, Player_ai->target_objnum);
 
@@ -2314,7 +2269,8 @@ void hud_squadmsg_wing_command()
 	
 	default_orders.erase(CAPTURE_TARGET_ITEM); // we cannot capture any target with a wing.
 
-	Num_menu_items = 0;
+	MsgItems.clear();
+	Rebuild_MsgItems = false;
 	shipnum = wingp->ship_index[wingp->special_ship];
 	Assertion(shipnum >= 0, "Special ship (%d) for wing '%s' has a negative ship_index (%d). This should not happen; get a coder!\n", wingp->special_ship, wingp->name, shipnum);
 	orders = Ships[shipnum].orders_accepted;		// get the orders that the wing leader will accept
@@ -2322,24 +2278,21 @@ void hud_squadmsg_wing_command()
 	for ( size_t order_id : default_orders ) {
 		// add the set of default orders to the comm menu.  We will currently allow all messages
 		// to be available in the wing.
-		Assert ( Num_menu_items < MAX_MENU_ITEMS );
-		MsgItems[Num_menu_items].text = Player_orders[order_id].localized_name;
-		MsgItems[Num_menu_items].instance = (int)order_id;
-		MsgItems[Num_menu_items].active = 0;
+		MsgItems.push_back({(int)order_id, 0, Player_orders[order_id].localized_name});
 
 		// possibly grey out the menu item depending on whether or not the "wing" will accept this order
 		// the "wing" won't accept the order if the first ship in the wing doesn't accept it.
 		if ( orders.find(order_id) != orders.end() )
-			MsgItems[Num_menu_items].active = 1;
+			MsgItems.back().active = 1;
 
 		// do some other checks to possibly gray out other items.
 		// if no target, remove any items which are associated with the players target
 		if ( !hud_squadmsg_is_target_order_valid((int)order_id, 0, true) )
-			MsgItems[Num_menu_items].active = 0;
+			MsgItems.back().active = 0;
 
 		// if no ship in the wing can depart then gray out the departure order
 		if (order_id == DEPART_ITEM) {
-			if (MsgItems[Num_menu_items].active > 0) {
+			if (MsgItems.back().active > 0) {
 				int active = 0;
 				for (int i = 0; i < wingp->current_count; i++) {
 					if (hud_squadmsg_ship_order_valid(wingp->ship_index[i], (int)order_id)) {
@@ -2347,12 +2300,9 @@ void hud_squadmsg_wing_command()
 						break;
 					}
 				}
-				MsgItems[Num_menu_items].active = active;
+				MsgItems.back().active = active;
 			}
 		}
-
-		Num_menu_items++;
-	
 	}
 
 	
@@ -2904,15 +2854,17 @@ void HudGaugeSquadMessage::render(float  /*frametime*/, bool config)
 		renderString(x + Header_offsets[0], y + fl2i(Header_offsets[1] * scale), title, scale, config);
 	}
 
+	int msg_items_size = sz2i(MsgItems.size());
+
 	int nitems;
 	if (!config) {
-		if (Num_menu_items < MAX_MENU_DISPLAY)
-			nitems = Num_menu_items;
+		if (msg_items_size < MAX_MENU_DISPLAY)
+			nitems = msg_items_size;
 		else {
 			if (First_menu_item == 0) // First_menu_item == 0 means first page of items
 				nitems = MAX_MENU_DISPLAY;
-			else if ((Num_menu_items - First_menu_item) <= MAX_MENU_DISPLAY) // check if remaining items fit on one page
-				nitems = Num_menu_items - First_menu_item;
+			else if (msg_items_size - First_menu_item <= MAX_MENU_DISPLAY) // check if remaining items fit on one page
+				nitems = msg_items_size - First_menu_item;
 			else {
 				nitems = MAX_MENU_DISPLAY;
 			}
@@ -2953,9 +2905,6 @@ void HudGaugeSquadMessage::render(float  /*frametime*/, bool config)
 				XSTR("Abort Rearm", 298)
 			};
 			strcpy_s(text, temp_comm_order_types[i]);
-			if (Hide_main_rearm_items_in_comms_gauge && (i == TYPE_REPAIR_REARM_ITEM || i == TYPE_REPAIR_REARM_ABORT_ITEM)) {
-				MsgItems[First_menu_item + i].active = -1;
-			}
 		}
 
 		// blit the background
@@ -2976,7 +2925,10 @@ void HudGaugeSquadMessage::render(float  /*frametime*/, bool config)
 			setGaugeColor(HUD_C_DIM, config);
 		}
 
-		if (MsgItems[First_menu_item + i].active >= 0) {
+		bool item_visible = config
+			? (!Hide_main_rearm_items_in_comms_gauge) || ((i != TYPE_REPAIR_REARM_ITEM) && (i != TYPE_REPAIR_REARM_ABORT_ITEM))
+			: (MsgItems[First_menu_item + i].active >= 0);
+		if (item_visible) {
 			// first print an icon to indicate selected item
 			item_num = (i + 1) % MAX_MENU_DISPLAY;
 			if (isSelectedItem && Display_selector) {
@@ -3001,7 +2953,7 @@ void HudGaugeSquadMessage::render(float  /*frametime*/, bool config)
 
 	// maybe draw an extra line in to make room for [pgdn], or for the 'no active items'
 	// display
-	if (!config && (!messaging_allowed || none_valid || ((First_menu_item + nitems) < Num_menu_items) || (Msg_shortcut_command != -1))) {
+	if (!config && (!messaging_allowed || none_valid || ((First_menu_item + nitems) < msg_items_size) || (Msg_shortcut_command != -1))) {
 		// blit the background
 		setGaugeColor(HUD_C_NONE, config);
 		if ( Mbox_gauge[1].first_frame >= 0 ) {		
@@ -3026,7 +2978,7 @@ void HudGaugeSquadMessage::render(float  /*frametime*/, bool config)
 		renderPrintf(x + fl2i(Pgup_offsets[0] * scale), y + fl2i(Pgup_offsets[1] * scale), scale, config, "%s", XSTR( "[pgup]", 312) );
 	}
 
-	if ( (First_menu_item + nitems) < Num_menu_items ) {
+	if ( (First_menu_item + nitems) < msg_items_size ) {
 		renderPrintf(x + fl2i(Pgdn_offsets[0] * scale), y + fl2i(Pgdn_offsets[1] * scale), scale, config, "%s", XSTR( "[pgdn]", 313));
 	}
 

code/hud/hudsquadmsg.h

@@ -62,7 +62,6 @@ struct reinforcements;
 // used for Message box gauge
 #define NUM_MBOX_FRAMES		3
 
-#define MAX_MENU_ITEMS 50   // max number of items in the menu
 #define MAX_MENU_DISPLAY 10 // max number that can be displayed
 
 // following are defines and character strings that are used as part of messaging mode
@@ -83,8 +82,7 @@ typedef struct mmode_item {
 } mmode_item;
 
 extern char Squad_msg_title[256];
-extern mmode_item MsgItems[MAX_MENU_ITEMS];
-extern int Num_menu_items; // number of items for a message menu
+extern SCP_vector<mmode_item> MsgItems;
 
 typedef struct player_order {
 private:
@@ -192,8 +190,7 @@ void hud_enemymsg_toggle();						// debug function to allow messaging of enemies
 void hud_squadmsg_do_mode( int mode );
 
 // functions for menu item selection with simple up/down/select buttons
-void hud_squadmsg_selection_move_down();
-void hud_squadmsg_selection_move_up();
+void hud_squadmsg_selection_move( bool up );
 void hud_squadmsg_selection_select();
 
 // Added for checking message validity - Mjn

code/hud/hudtarget.cpp

@@ -6775,10 +6775,9 @@ void HudGaugeWeapons::render(float /*frametime*/, bool config)
 			renderPrintfWithGauge(x + fl2i(Weapon_sunlinked_offset_x * scale), name_y, EG_NULL, scale, config, "%c", Weapon_link_icon);
 
 			// indicate if this is linked
-			// don't draw the link indicator if the fire can't be fired link.
-			// the link flag is ignored rather than cleared so the player can cycle past a no-doublefire weapon without the setting being cleared
-			if (!config && Player_ship->flags[Ship::Ship_Flags::Secondary_dual_fire] && !wip->wi_flags[Weapon::Info_Flags::No_doublefire] &&
-					!The_mission.ai_profile->flags[AI::Profile_Flags::Disable_player_secondary_doublefire] ) {
+			// don't draw the link indicator if this bank can't actually fire linked.
+			// the link flag is ignored rather than cleared so the player can cycle past an incapable bank or weapon without the setting being cleared
+			if (!config && Player_ship->flags[Ship::Ship_Flags::Secondary_dual_fire] && ship_secondary_bank_can_dual_fire(Player_ship, i)) {
 				renderPrintfWithGauge(x + fl2i(Weapon_slinked_offset_x * scale), name_y, EG_NULL, scale, config, "%c", Weapon_link_icon);
 			}
 
@@ -7732,10 +7731,9 @@ void HudGaugeSecondaryWeapons::render(float /*frametime*/, bool config)
 			renderPrintfWithGauge(position[0] + _sunlinked_offset_x, position[1] + text_y_offset, EG_NULL, 1.0f, config, "%c", Weapon_link_icon);
 
 			// indicate if this is linked
-			// don't draw the link indicator if the fire can't be fired link.
-			// the link flag is ignored rather than cleared so the player can cycle past a no-doublefire weapon without the setting being cleared
-			if ( Player_ship->flags[Ship::Ship_Flags::Secondary_dual_fire] && !wip->wi_flags[Weapon::Info_Flags::No_doublefire] &&
-					!The_mission.ai_profile->flags[AI::Profile_Flags::Disable_player_secondary_doublefire] ) {
+			// don't draw the link indicator if this bank can't actually fire linked.
+			// the link flag is ignored rather than cleared so the player can cycle past an incapable bank or weapon without the setting being cleared
+			if ( Player_ship->flags[Ship::Ship_Flags::Secondary_dual_fire] && ship_secondary_bank_can_dual_fire(Player_ship, i) ) {
 				renderPrintfWithGauge(position[0] + _slinked_offset_x, position[1] + text_y_offset, EG_NULL, 1.0f, config, "%c", Weapon_link_icon);
 			}
 
@@ -7945,7 +7943,7 @@ void HudGaugeHardpoints::render(float /*frametime*/, bool config)
 
 					model_render_params weapon_render_info;
 
-					if ( swp->current_secondary_bank == i && ( swp->secondary_next_slot[i] == k || ( swp->secondary_next_slot[i]+1 == k && sp->flags[Ship::Ship_Flags::Secondary_dual_fire] ) ) ) {
+					if ( swp->current_secondary_bank == i && ( swp->secondary_next_slot[i] == k || ( swp->secondary_next_slot[i]+1 == k && sp->flags[Ship::Ship_Flags::Secondary_dual_fire] && ship_secondary_bank_can_dual_fire(sp, i) ) ) ) {
 						weapon_render_info.set_color(Color_bright_blue);
 					} else {
 						weapon_render_info.set_color(Color_bright_white);

code/io/keycontrol.cpp

@@ -1931,19 +1931,22 @@ int button_function_critical(int n, net_player *p = NULL)
 				break;
 			}
 
-			polymodel *pm = model_get(Ship_info[Ships[objp->instance].ship_info_index].model_num);
-
-			int firepoints = pm->missile_banks[Ships[objp->instance].weapons.current_secondary_bank].num_slots;
-
-            if (Ships[objp->instance].flags[Ship::Ship_Flags::Secondary_dual_fire] || firepoints < 2) {
-                Ships[objp->instance].flags.remove(Ship::Ship_Flags::Secondary_dual_fire);
+			if (Ships[objp->instance].flags[Ship::Ship_Flags::Secondary_dual_fire]) {
+				Ships[objp->instance].flags.remove(Ship::Ship_Flags::Secondary_dual_fire);
 				if(at_self) {
 					HUD_sourced_printf(HUD_SOURCE_HIDDEN, "%s", XSTR( "Secondary weapon set to normal fire mode", 34));
 					snd_play( gamesnd_get_game_sound(ship_get_sound(Player_obj, GameSounds::SECONDARY_CYCLE)) );
 					hud_gauge_popup_start(HUD_WEAPONS_GAUGE);
 				}
+			} else if (!ship_secondary_bank_can_dual_fire(&Ships[objp->instance], Ships[objp->instance].weapons.current_secondary_bank)) {
+				// leave the dual fire preference alone; it is ignored rather than cleared for banks that can't use it
+				if(at_self) {
+					HUD_sourced_printf(HUD_SOURCE_HIDDEN, "%s", XSTR( "This secondary bank cannot fire dual missiles", 1932));
+					gamesnd_play_iface(InterfaceSounds::GENERAL_FAIL);
+					hud_gauge_popup_start(HUD_WEAPONS_GAUGE);
+				}
 			} else {
-                Ships[objp->instance].flags.set(Ship::Ship_Flags::Secondary_dual_fire);
+				Ships[objp->instance].flags.set(Ship::Ship_Flags::Secondary_dual_fire);
 				if(at_self) {
 					HUD_sourced_printf(HUD_SOURCE_HIDDEN, "%s", XSTR( "Secondary weapon set to dual fire mode", 35));
 					snd_play( gamesnd_get_game_sound(ship_get_sound(Player_obj, GameSounds::SECONDARY_CYCLE)) );
@@ -2905,11 +2908,11 @@ int button_function(int n)
 			break;
 			 
 		case COMMS_MENU_MOVE_DOWN:
-			hud_squadmsg_selection_move_down();
+			hud_squadmsg_selection_move(false);
 			break;
 
 		case COMMS_MENU_MOVE_UP:
-			hud_squadmsg_selection_move_up();
+			hud_squadmsg_selection_move(true);
 			break;
 
 		case COMMS_MENU_SELECT:

code/io/mouse.cpp

@@ -97,10 +97,6 @@ const std::shared_ptr<scripting::Hook<>> OnMouseWheelHook = scripting::Hook<>::F
 		{"MouseWheelX", "number", "Positive if moved right, negative if moved left."},
 	});
 
-#define SCALE_MOUSE_TO_WINDOW(x, y, op) \
-	static_cast<decltype(x)>(Cmdline_window_res ? static_cast<float>(x) op (static_cast<float>(gr_screen.max_w) / static_cast<float>(Cmdline_window_res->first)) : x), \
-	static_cast<decltype(y)>(Cmdline_window_res ? static_cast<float>(y) op (static_cast<float>(gr_screen.max_h) / static_cast<float>(Cmdline_window_res->second)) : y)
-
 namespace
 {
 	bool mouse_key_event_handler(const SDL_Event& e)
@@ -139,7 +135,16 @@ namespace
 			return false;
 		}
 
-		mouse_event(SCALE_MOUSE_TO_WINDOW(e.motion.x, e.motion.y, *), SCALE_MOUSE_TO_WINDOW(e.motion.xrel, e.motion.yrel, *));
+		// SDL reports these in window pixels, the rest of the engine works in render pixels
+		float x = i2fl(e.motion.x);
+		float y = i2fl(e.motion.y);
+		gr_window_to_render_pos(x, y);
+
+		float dx = i2fl(e.motion.xrel);
+		float dy = i2fl(e.motion.yrel);
+		gr_window_to_render_pos(dx, dy);
+
+		mouse_event(fl2i(x), fl2i(y), fl2i(dx), fl2i(dy));
 
 		return true;
 	}
@@ -541,7 +546,11 @@ void mouse_get_wheel_delta(int* dx, int* dy)
 void mouse_force_pos(int x, int y)
 {
 	if (os_foreground()) {  // only mess with windows's mouse if we are in control of it
-		SDL_WarpMouseInWindow(os::getSDLMainWindow(), SCALE_MOUSE_TO_WINDOW(x, y, /));
+		float fx = i2fl(x);
+		float fy = i2fl(y);
+		gr_render_to_window_pos(fx, fy);
+
+		SDL_WarpMouseInWindow(os::getSDLMainWindow(), fl2i(fx), fl2i(fy));
 	}
 }
 
@@ -628,13 +637,16 @@ int mouse_get_pos_unscaled( int *xpos, int *ypos )
 
 void mouse_get_real_pos(int *mx, int *my)
 {
-	SDL_GetMouseState(mx, my);
-	if (Cmdline_window_res) {
-		if (mx)
-			*mx *= gr_screen.max_w / Cmdline_window_res->first;
-		if (my)
-			*my *= gr_screen.max_h / Cmdline_window_res->second;
-	}
+	int x = 0;
+	int y = 0;
+	SDL_GetMouseState(&x, &y);
+
+	float fx = i2fl(x);
+	float fy = i2fl(y);
+	gr_window_to_render_pos(fx, fy);
+
+	if (mx) *mx = fl2i(fx);
+	if (my) *my = fl2i(fy);
 }
 
 void mouse_set_pos(int xpos, int ypos)

code/localization/localize.cpp

@@ -64,7 +64,7 @@ bool *Lcl_unexpected_tstring_check = nullptr;
 // NOTE: with map storage of XSTR strings, the indexes no longer need to be contiguous,
 // but internal strings should still increment XSTR_SIZE to avoid collisions.
 // retail XSTR_SIZE = 1570
-// #define XSTR_SIZE	1932 // This is the next available ID
+// #define XSTR_SIZE	1933 // This is the next available ID
 
 // struct to allow for strings.tbl-determined x offset
 // offset is 0 for english, by default

code/mission/missionbriefcommon.h

@@ -20,6 +20,8 @@
 #include "hud/hud.h"
 #include "utils/unicode.h"
 
+#include <utility>
+
 #define MAX_TEXT_STREAMS	2		// how many concurrent streams of text can be displayed
 
 // ------------------------------------------------------------------------
@@ -163,15 +165,54 @@ public:
 	bool		draw_grid;
 	color		grid_color;
 
-	brief_stage( ) 
-		: camera_time( 0 ), flags( 0 ), formula( -1 ), num_icons(0), icons(nullptr), num_lines(0), lines(nullptr),
-		  draw_grid(true)
-	{ 
+	brief_stage( )
+	{
+		reset();
+	}
+
+	// Restore this stage to its default-constructed state.  Does not free
+	// the icons/lines buffers; the caller is responsible for those.
+	void reset()
+	{
+		text.clear();
 		voice[ 0 ] = 0;
 		camera_pos = vmd_zero_vector;
 		camera_orient = vmd_identity_matrix;
+		camera_time = 0;
+		flags = 0;
+		formula = -1;
+		num_icons = 0;
+		icons = nullptr;
+		num_lines = 0;
+		lines = nullptr;
+		draw_grid = true;
 		grid_color = Color_briefing_grid;
 	}
+
+	// icons and lines are raw owning pointers, so the memberwise copy/move
+	// operations would alias those buffers across two stages; use swap() to
+	// rearrange stages, or copy the buffer contents explicitly
+	brief_stage(const brief_stage&) = delete;
+	brief_stage& operator=(const brief_stage&) = delete;
+	brief_stage(brief_stage&&) = delete;
+	brief_stage& operator=(brief_stage&&) = delete;
+
+	friend void swap(brief_stage& a, brief_stage& b)
+	{
+		std::swap(a.text, b.text);
+		std::swap(a.voice, b.voice);
+		std::swap(a.camera_pos, b.camera_pos);
+		std::swap(a.camera_orient, b.camera_orient);
+		std::swap(a.camera_time, b.camera_time);
+		std::swap(a.flags, b.flags);
+		std::swap(a.formula, b.formula);
+		std::swap(a.num_icons, b.num_icons);
+		std::swap(a.icons, b.icons);
+		std::swap(a.num_lines, b.num_lines);
+		std::swap(a.lines, b.lines);
+		std::swap(a.draw_grid, b.draw_grid);
+		std::swap(a.grid_color, b.grid_color);
+	}
 };
 
 class debrief_stage

code/mission/missionparse.cpp

@@ -4797,8 +4797,26 @@ int parse_wing_create_ships( wing *wingp, int num_to_create, bool force_create,
 		wingp->total_arrived_count++;
 		if (wingp->num_waves > 1)
 		{
+			char pre_bash_name[NAME_LENGTH];
+			strcpy_s(pre_bash_name, p_objp->name);
+
 			wing_bash_ship_name(p_objp, wingp, wingp->total_arrived_count + wingp->red_alert_skipped_ships);
 
+			// if the bash renamed this parse object (which can happen if the ship was saved using the legacy
+			// hash format), re-key its not-yet-present registry entry so that the old name doesn't shadow
+			// lookups of the new name.  (For subsequent waves, the pre-bash name belongs to a previous wave's
+			// ship, whose entry must keep its key; the status and parse object checks exclude that case.)
+			if (stricmp(pre_bash_name, p_objp->name) != 0)
+			{
+				auto ship_it = Ship_registry_map.find(pre_bash_name);
+				if (ship_it != Ship_registry_map.end()
+					&& Ship_registry[ship_it->second].status == ShipStatus::NOT_YET_PRESENT
+					&& Ship_registry[ship_it->second].pobj_num == POBJ_INDEX(p_objp))
+				{
+					ship_registry_rename(ship_it->second, p_objp->name, true);
+				}
+			}
+
 			// subsequent waves of ships will not be in the ship registry, so add them
 			if (!ship_registry_exists(p_objp->name))
 			{
@@ -4810,6 +4828,12 @@ int parse_wing_create_ships( wing *wingp, int num_to_create, bool force_create,
 				Ship_registry_map[p_objp->name] = static_cast<int>(Ship_registry.size() - 1);
 			}
 		}
+		// make sure we still handle wing display names in a single-wave wing
+		else if (wingp->has_display_name())
+		{
+			wing_bash_ship_name(p_objp->display_name, wingp->get_display_name(), wingp->total_arrived_count + wingp->red_alert_skipped_ships);
+			p_objp->flags.set(Mission::Parse_Object_Flags::SF_Has_display_name);
+		}
 
 		// also, if multiplayer, set the parse object's net signature to be wing's net signature
 		// base + total_arrived_count (before adding 1)
@@ -8022,6 +8046,17 @@ int mission_parse_get_multi_mission_info( const char *filename )
 	return The_mission.num_players;
 }
 
+static p_object *mission_parse_get_arrival_ship_sub(const char *name)
+{
+	for (auto p_objp : list_range(&Ship_arrival_list))
+	{
+		if (!stricmp(p_objp->name, name))
+			return p_objp;	// still on the arrival list
+	}
+
+	return nullptr;
+}
+
 /**
  * @brief				Returns the parse object on the ship arrival list associated with the given name.
  * @param[in] name		The name of the object
@@ -8032,18 +8067,18 @@ int mission_parse_get_multi_mission_info( const char *filename )
  */
 p_object *mission_parse_get_arrival_ship(const char *name)
 {
-	p_object *p_objp;
-
 	if (name == nullptr)
 		return nullptr;
 
-	for (p_objp = GET_FIRST(&Ship_arrival_list); p_objp != END_OF_LIST(&Ship_arrival_list); p_objp = GET_NEXT(p_objp))
-	{
-		if (!stricmp(p_objp->name, name)) 
-		{
-			return p_objp;	// still on the arrival list
-		}
-	}
+	// try the normal lookup
+	auto p_objp = mission_parse_get_arrival_ship_sub(name);
+	if (p_objp)
+		return p_objp;
+
+	// also search for ship names hashed using the legacy format
+	SCP_string legacy_hashed;
+	if (wing_bash_legacy_hashed_ship_name(legacy_hashed, name))
+		return mission_parse_get_arrival_ship_sub(legacy_hashed.c_str());
 
 	return nullptr;
 }

code/mission/missionparse.h

@@ -530,7 +530,7 @@ public:
 
 // Goober5000 - this is now dynamic
 extern SCP_vector<p_object> Parse_objects;
-#define POBJ_INDEX(pobjp) (int)(pobjp - &Parse_objects[0])	// yes, this arithmetic is valid :D
+#define POBJ_INDEX(pobjp) (static_cast<int>((pobjp)-Parse_objects.data()))
 
 extern p_object Support_ship_pobj, *Arriving_support_ship;
 extern p_object Ship_arrival_list;

code/missioneditor/campaignsave.cpp

@@ -268,7 +268,7 @@ int Fred_campaign_save::save_campaign_file(const char* pathname, const SCP_vecto
 						fout(")");
 					}
 					if (mission_fork) {
-						fout("Option: ", Campaign.missions[link.to].name);
+						fout("\n+Option: %s", Campaign.missions[link.to].name);
 					}
 				}
 			}

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

@@ -2563,7 +2563,7 @@ int Fred_mission_save::save_mission_info()
 		fout("\n+Flags:");
 	}
 
-	fout(" %" PRIu64, The_mission.flags.to_u64());
+	fout(" " UINT64_T_ARG, The_mission.flags.to_u64());
 
 	// maybe write out Nebula values
 	if (The_mission.flags[Mission::Mission_Flags::Fullneb]) {
@@ -2858,22 +2858,29 @@ int Fred_mission_save::save_mission_info()
 	// this is compatible with non-SCP variants - Goober5000
 	fout(" %d", (The_mission.support_ships.max_support_ships == 0) ? 1 : 0);
 
+	// version-gated support options; see also check_for_25_1_data()
+	auto more_support_options_version = gameversion::version(25, 1);
+
 	if (save_config.save_format != MissionFormat::RETAIL) {
-		if (optional_string_fred("+Disallow Support Rearm:")) {
-			parse_comments(2);
-		} else {
-			fout("\n+Disallow Support Rearm:");
+		if (The_mission.required_fso_version >= more_support_options_version) {
+			if (optional_string_fred("+Disallow Support Rearm:")) {
+				parse_comments(2);
+			} else {
+				fout("\n+Disallow Support Rearm:");
+			}
+
+			fout(" %d", The_mission.support_ships.disallow_rearm ? 1 : 0);
 		}
 
-		fout(" %d", The_mission.support_ships.disallow_rearm ? 1 : 0);
+		if (The_mission.required_fso_version >= more_support_options_version) {
+			if (optional_string_fred("+Allow Support Rearm Weapon Precedence:")) {
+				parse_comments(2);
+			} else {
+				fout("\n+Allow Support Rearm Weapon Precedence:");
+			}
 
-		if (optional_string_fred("+Allow Support Rearm Weapon Precedence:")) {
-			parse_comments(2);
-		} else {
-			fout("\n+Allow Support Rearm Weapon Precedence:");
+			fout(" %d", The_mission.support_ships.allow_rearm_weapon_precedence ? 1 : 0);
 		}
-
-		fout(" %d", The_mission.support_ships.allow_rearm_weapon_precedence ? 1 : 0);
 	}
 
 	// here be WMCoolmon's hull and subsys repair stuff
@@ -2892,12 +2899,14 @@ int Fred_mission_save::save_mission_info()
 		}
 		fout(" %f", The_mission.support_ships.max_subsys_repair_val);
 
-		if (optional_string_fred("+Support Rearm Pool From Loadout:")) {
-			parse_comments(1);
-		} else {
-			fout("\n+Support Rearm Pool From Loadout:");
+		if (The_mission.required_fso_version >= more_support_options_version) {
+			if (optional_string_fred("+Support Rearm Pool From Loadout:")) {
+				parse_comments(1);
+			} else {
+				fout("\n+Support Rearm Pool From Loadout:");
+			}
+			fout(" %d", The_mission.support_ships.rearm_pool_from_loadout ? 1 : 0);
 		}
-		fout(" %d", The_mission.support_ships.rearm_pool_from_loadout ? 1 : 0);
 	}
 
 	if (Mission_all_attack) {
@@ -3602,7 +3611,7 @@ int Fred_mission_save::save_objects()
 			fout(" %s", save_config.fred_alt_names[i]);
 		}
 
-		Assertion(save_config.fred_callsigns != nullptr, "save_config.fred_alt_names is null!");
+		Assertion(save_config.fred_callsigns != nullptr, "save_config.fred_callsigns is null!");
 
 		// optional callsign
 		if (save_config.save_format != MissionFormat::RETAIL && strlen(save_config.fred_callsigns[i])) {
@@ -4390,8 +4399,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) {
@@ -4509,7 +4517,23 @@ int Fred_mission_save::save_players()
 
 		fout(")");
 
-		if (save_config.save_format != MissionFormat::RETAIL && !The_mission.support_ships.rearm_pool_from_loadout) {
+		// version-gated support options; see also check_for_25_1_data()
+		auto more_support_options_version = gameversion::version(25, 1);
+
+		bool has_rearm_pool_entries = false;
+		if (save_config.save_format != MissionFormat::RETAIL && The_mission.required_fso_version >= more_support_options_version && !The_mission.support_ships.rearm_pool_from_loadout) {
+			for (j = 0; j < weapon_info_size(); j++) {
+				// mirror the skip conditions used when writing the pool below
+				if (!Weapon_info[j].wi_flags[Weapon::Info_Flags::Player_allowed] || Weapon_info[j].disallow_rearm) {
+					continue;
+				}
+				if (The_mission.support_ships.rearm_weapon_pool[i][j] != -1) {
+					has_rearm_pool_entries = true;
+					break;
+				}
+			}
+		}
+		if (has_rearm_pool_entries) {
 			if (optional_string_fred("+Support Rearm Pool:", "$Starting Shipname:")) {
 				parse_comments(2);
 			} else {

code/missionui/missionbrief.cpp

@@ -821,7 +821,7 @@ void brief_compact_stages()
 
 			Briefing->stages[num].num_icons = 0;
 			for ( i = num+1; i < Briefing->num_stages; i++ ) {
-				Briefing->stages[i-1] = Briefing->stages[i];
+				swap(Briefing->stages[i-1], Briefing->stages[i]);
 			}
 			Briefing->num_stages--;
 			continue;
@@ -832,7 +832,7 @@ void brief_compact_stages()
 	// completely clear out the old entries (if any) so we don't access them by mistake - taylor
 	if ( before > Briefing->num_stages ) {
 		for ( i = Briefing->num_stages; i < before; i++ ) {
-			Briefing->stages[i] = brief_stage();
+			Briefing->stages[i].reset();
 		}
 	}
 }

code/missionui/missionscreencommon.cpp

@@ -1560,8 +1560,12 @@ int restore_wss_data(ubyte *data)
 
 void draw_model_icon(int model_id, uint64_t flags, int x, int y, int w, int h, ship_info* sip, weapon_info* wip, float zoom_multiplier, int resize_mode)
 {
+	// Can't draw a non-model
+	if (model_id < 0)
+		return;
+
 	lighting_profiles::set_non_mission_profile non_mission_lighting_profile;
-	
+
 	matrix	object_orient	= IDENTITY_MATRIX;
 	angles rot_angles = vmd_zero_angles;
 

code/missionui/missionshipchoice.cpp

@@ -1697,6 +1697,9 @@ void draw_ship_icon_with_number(int screen_offset, int ship_class)
 			ss_icon->model_index = model_load(sip, true);
 			mprintf(("SL WARNING: Had to attempt to page in model for %s paged in manually! Result: %d\n", sip->name, ss_icon->model_index));
 		}
+		// the icon bitmap can be invalid without a color having been chosen, e.g. when the icon ani is missing frames
+		if (color_to_draw == nullptr)
+			color_to_draw = &Icon_colors[ICON_FRAME_NORMAL];
 		gr_set_color_fast(color_to_draw);
 
 		graphics::line_draw_list line_draw_list;
@@ -2184,6 +2187,7 @@ void draw_wing_block(int wb_num, int hot_slot, int selected_slot, int class_sele
 		GR_DEBUG_SCOPE("Single ship");
 
 		bitmap_to_draw = -1;
+		color_to_draw = nullptr;
 		ws = &wb->ss_slots[i];
 		slot_index = wb_num*MAX_WING_SLOTS + i;
 

code/missionui/missionweaponchoice.cpp

@@ -3132,6 +3132,9 @@ void wl_render_icon(int index, int x, int y, int num, int draw_num_flag, int hot
 	}
 	else
 	{
+		// the icon bitmap can be invalid without a color having been chosen, e.g. when the icon ani is missing frames
+		if (color_to_draw == nullptr)
+			color_to_draw = &Icon_colors[ICON_FRAME_NORMAL];
 		gr_set_color_fast(color_to_draw);
 
 		graphics::line_draw_list line_draw_list;

code/mod_table/mod_table.cpp

@@ -170,6 +170,7 @@ bool Use_new_scanning_behavior;
 bool Lua_API_returns_nil_instead_of_invalid_object;
 bool Dont_show_callsigns_in_escort_list;
 bool Hide_main_rearm_items_in_comms_gauge;
+bool Always_show_selected_item_in_comms_gauge;
 bool Fix_scripted_velocity;
 color Overhead_line_colors[MAX_SHIP_SECONDARY_BANKS];
 bool Preload_briefing_icon_models;
@@ -495,6 +496,10 @@ void parse_mod_table(const char *filename)
 				stuff_boolean(&Hide_main_rearm_items_in_comms_gauge);
 			}
 
+			if (optional_string("$Always show selected item in Comms Gauge:")) {
+				stuff_boolean(&Always_show_selected_item_in_comms_gauge);
+			}
+
 			optional_string("#SEXP SETTINGS");
 
 			if (optional_string("$Loop SEXPs Then Arguments:")) {
@@ -1882,6 +1887,7 @@ void mod_table_reset()
 	Use_new_scanning_behavior = false;
 	Lua_API_returns_nil_instead_of_invalid_object = false;
 	Dont_show_callsigns_in_escort_list = false;
+	Always_show_selected_item_in_comms_gauge = false;
 	Hide_main_rearm_items_in_comms_gauge = false;
 	Fix_scripted_velocity = false;
 	// These colors were taken from missionscreencommon.cpp line 591 which

code/mod_table/mod_table.h

@@ -192,6 +192,7 @@ extern bool Use_new_scanning_behavior;
 extern bool Lua_API_returns_nil_instead_of_invalid_object;
 extern bool Dont_show_callsigns_in_escort_list;
 extern bool Hide_main_rearm_items_in_comms_gauge;
+extern bool Always_show_selected_item_in_comms_gauge;
 extern bool Fix_scripted_velocity;
 extern color Overhead_line_colors[MAX_SHIP_SECONDARY_BANKS];
 extern bool Preload_briefing_icon_models;

code/model/modelread.cpp

@@ -35,6 +35,7 @@
 #include "model/modelreplace.h"
 #include "model/modelsinc.h"
 #include "parse/parselo.h"
+#include "prop/prop.h"
 #include "render/3dinternal.h"
 #include "ship/ship.h"
 #include "starfield/starfield.h"
@@ -312,6 +313,16 @@ void model_unload(int modelnum, int force)
 		}
 	}
 
+	// and props, for the same reason: props_level_close() only clears the prop
+	// instances, so without this the class-level handle survives the model being
+	// freed and the next placement of that prop class reads a slot that has since
+	// been reused by an unrelated model.
+	for (auto& pip : Prop_info) {
+		if (pm->id == pip.model_num) {
+			pip.model_num = -1;
+		}
+	}
+
 	model_free(pm);
 
 	Polygon_models[num] = NULL;	
@@ -1300,7 +1311,7 @@ void determine_submodel_movement(bool is_rotation, const char *filename, bsp_inf
 
 		if (in(p, props, axis_string))
 		{
-			if (get_user_vec3d_value(p + 20, movement_axis, true, sm->name, filename))
+			if (get_user_vec3d_value(p + strlen(axis_string), movement_axis, true, sm->name, filename))
 			{
 				if (!fl_near_zero(vm_vec_mag(movement_axis)))
 					vm_vec_normalize(movement_axis);
@@ -4075,7 +4086,7 @@ void submodel_canonicalize_translation(bsp_info *sm, submodel_instance *smi)
 	smi->canonical_prev_offset = smi->canonical_offset;
 
 	// get the vector
-	switch (sm->rotation_axis_id)
+	switch (sm->translation_axis_id)
 	{
 		case MOVEMENT_AXIS_X:
 			vm_vec_copy_scale(&smi->canonical_offset, &vmd_x_vector, smi->cur_offset);
@@ -4427,6 +4438,10 @@ bool model_rotate_gun(const object *objp, const polymodel *pm, const polymodel_i
 		vm_vec_sub(&dir, &planar_dst, &world_pos);
 		vm_vec_normalize(&dir);
 		desired_base_angle = vm_vec_delta_ang_norm(&rotated_vec, &dir, &world_axis);
+		// for ventral turrets without custom matrixes
+		if (vm_vec_dot(&base_sm->frame_of_reference.vec.uvec, &turret->turret_norm) < 0.0f) {
+			desired_base_angle = PI + desired_base_angle;
+		}
 
 		//------------
 		// Pretend the base is pointing directly at the target

code/network/multi.h

@@ -134,7 +134,7 @@ class player;
 //
 
 // netplayer management
-#define NET_PLAYER_INDEX(np)	(static_cast<int>(np-Net_players))
+#define NET_PLAYER_INDEX(np)	(static_cast<int>((np)-Net_players))
 #define NET_PLAYER_NUM(np)		(NET_PLAYER_INDEX(np))
 #define MY_NET_PLAYER_NUM		(NET_PLAYER_INDEX(Net_player))
 

code/network/multi_interpolate.cpp

@@ -165,8 +165,10 @@ void interpolation_manager::interpolate_main(vec3d* pos, matrix* ori, physics_in
 // correct the ship record for player ships when an up to date packet comes in.
 void interpolation_manager::reinterpolate_previous(TIMESTAMP stamp, int prev_packet_index, int next_packet_index,  vec3d& position, matrix& orientation, vec3d& velocity, vec3d& rotational_velocity)
 {
-	// calc what the timing was previously.
-	float numerator = static_cast<float>(stamp.value()) - static_cast<float>(_packets[prev_packet_index].remote_missiontime);
+	// calc what the timing was previously.  The caller hands us an absolute timestamp, 
+	// but remote_missiontime is relative to mission start, so drop the start time before comparing.
+	auto local_time = static_cast<float>(stamp.value() - Multi_Timing_Info.get_mission_start_time());
+	float numerator = local_time - static_cast<float>(_packets[prev_packet_index].remote_missiontime);
 	float denominator = static_cast<float>(_packets[next_packet_index].remote_missiontime) - static_cast<float>(_packets[prev_packet_index].remote_missiontime);
 
 	denominator = (denominator > 0.05f) ? denominator : 0.05f;

code/network/multi_obj.cpp

@@ -1223,6 +1223,17 @@ int multi_oo_pack_client_data(ubyte *data, ship* shipp)
 	return packet_size;
 }
 
+// vm_extract_angles_matrix_alternate returns angles in the range -PI..PI, but the subsystem list packer
+// encodes them as an unsigned fraction of a full rotation, so wrap negatives around before sending.
+static float multi_oo_normalized_angle(float angle)
+{
+	if (angle < 0.0f) {
+		angle += PI2;
+	}
+
+	return angle / PI2;
+}
+
 // pack the appropriate info into the data
 #define PACK_PERCENT(v) { std::uint8_t upercent; if(v < 0.0f){v = 0.0f;} upercent = (v * 255.0f) <= 255.0f ? (std::uint8_t)(v * 255.0f) : (std::uint8_t)255; memcpy(data + packet_size + header_bytes, &upercent, sizeof(std::uint8_t)); packet_size++; }
 #define PACK_BYTE(v) { memcpy( data + packet_size + header_bytes, &v, 1 ); packet_size += 1; }
@@ -1411,32 +1422,32 @@ int multi_oo_pack_data(net_player *pl, object *objp, ushort oo_flags, ubyte *dat
 				// here we're checking to see if the subsystems rotated enough to send.
 				if (angs_1 != nullptr && angs_1->b != Oo_info.player_frame_info[pl->player_id].last_sent[objp->net_signature].subsystem_1b[i]) {
 					flags[i] |= OO_SUBSYS_ROTATION_1b;
-					subsys_data.push_back(angs_1->b / PI2);
+					subsys_data.push_back(multi_oo_normalized_angle(angs_1->b));
 				}
 
 				if (angs_1 != nullptr && angs_1->h != Oo_info.player_frame_info[pl->player_id].last_sent[objp->net_signature].subsystem_1h[i]) {
 					flags[i] |= OO_SUBSYS_ROTATION_1h;
-					subsys_data.push_back(angs_1->h / PI2);
+					subsys_data.push_back(multi_oo_normalized_angle(angs_1->h));
 				}
 
 				if (angs_1 != nullptr && angs_1->p != Oo_info.player_frame_info[pl->player_id].last_sent[objp->net_signature].subsystem_1p[i]) {
 					flags[i] |= OO_SUBSYS_ROTATION_1p;
-					subsys_data.push_back(angs_1->p / PI2);
+					subsys_data.push_back(multi_oo_normalized_angle(angs_1->p));
 				}
 
 				if (angs_2 != nullptr && angs_2->b != Oo_info.player_frame_info[pl->player_id].last_sent[objp->net_signature].subsystem_2b[i]) {
 					flags[i] |= OO_SUBSYS_ROTATION_2b;
-					subsys_data.push_back(angs_2->b / PI2);
+					subsys_data.push_back(multi_oo_normalized_angle(angs_2->b));
 				}
 
 				if (angs_2 != nullptr && angs_2->h != Oo_info.player_frame_info[pl->player_id].last_sent[objp->net_signature].subsystem_2h[i]) {
 					flags[i] |= OO_SUBSYS_ROTATION_2h;
-					subsys_data.push_back(angs_2->h / PI2);
+					subsys_data.push_back(multi_oo_normalized_angle(angs_2->h));
 				}
 
 				if (angs_2 != nullptr && angs_2->p != Oo_info.player_frame_info[pl->player_id].last_sent[objp->net_signature].subsystem_2p[i]) {
 					flags[i] |= OO_SUBSYS_ROTATION_2p;
-					subsys_data.push_back(angs_2->p / PI2);
+					subsys_data.push_back(multi_oo_normalized_angle(angs_2->p));
 				}
 
 				// clang says deleting null pointer has no effect
@@ -1600,8 +1611,8 @@ int multi_oo_unpack_client_data(net_player* pl, ubyte* data, bool keep_data)
 
 	int offset = 0;
 
-	// read flag info
-	ushort in_flags;
+	// read flag info -- this is packed as a single byte, so it must be read back as one
+	ubyte in_flags;
 	memcpy(&in_flags, data, sizeof(ubyte));
 	offset++;
 
@@ -1895,7 +1906,7 @@ int multi_oo_unpack_data(net_player* pl, ubyte* data, int seq_num, int time_delt
 			full_physics = true;
 		}
 
-		int r5 = multi_pack_unpack_desired_vel_and_desired_rotvel(0, full_physics, data + offset, &pobjp->phys_info, &local_desired_vel);
+		int r5 = multi_pack_unpack_desired_vel_and_desired_rotvel(0, full_physics, data + offset, &new_phys_info, &local_desired_vel);
 		offset += r5;
 		// change it back to global coordinates.
 		vm_vec_unrotate(&new_phys_info.desired_vel, &local_desired_vel, &new_orient);

code/network/multimsgs.cpp

@@ -2646,7 +2646,7 @@ void process_ship_kill_packet( ubyte *data, header *hinfo )
 	}
 
 	// maybe set wash_killed
-	if (extra_death_info & EXTRA_DEATH_VAPORIZED) {
+	if (extra_death_info & EXTRA_DEATH_WASHED) {
 		Ships[sobjp->instance].wash_killed = 1;
 	}
 
@@ -7630,7 +7630,8 @@ void process_homing_weapon_info( ubyte *data, header *hinfo )
 	}
 
 	if (flags & HWIF_BIG_UPDATE) {
-		wp->creation_time = Missiontime + missile_lifetime;
+		// the sender packed the missile's age, so walk creation_time back from now to recover it
+		wp->creation_time = Missiontime - missile_lifetime;
 		weapon_objp->pos = missile_pos;
 		weapon_objp->orient = orient_in;
 		wp->launch_speed = launch_speed;

code/network/multiutil.cpp

@@ -3625,9 +3625,9 @@ int multi_pack_unpack_desired_vel_and_desired_rotvel( int write, bool full_physi
 			a = bitbuffer_get_signed(&buf,5);
 			b = bitbuffer_get_signed(&buf,5);
 			c = bitbuffer_get_signed(&buf,5);
-			pi->rotvel.xyz.x = pi->max_rotvel.xyz.x * i2fl(a)/15.0f;
-			pi->rotvel.xyz.y = pi->max_rotvel.xyz.y * i2fl(b)/15.0f;
-			pi->rotvel.xyz.z = pi->max_rotvel.xyz.z * i2fl(c)/15.0f;
+			pi->desired_rotvel.xyz.x = pi->max_rotvel.xyz.x * i2fl(a)/15.0f;
+			pi->desired_rotvel.xyz.y = pi->max_rotvel.xyz.y * i2fl(b)/15.0f;
+			pi->desired_rotvel.xyz.z = pi->max_rotvel.xyz.z * i2fl(c)/15.0f;
 
 		}
 

code/object/collidedebrisship.cpp

@@ -553,7 +553,7 @@ int collide_asteroid_prop(obj_pair* pair)
 			asteroid_hit_info.light = asteroid_objp;
 		}
 
-		hit = prop_check_collision(prop_objp, prop_objp, &hitpos, &asteroid_hit_info);
+		hit = prop_check_collision(prop_objp, asteroid_objp, &hitpos, &asteroid_hit_info);
 		if (hit)
 		{
 			bool ship_override = false, asteroid_override = false;

code/object/collideshipweapon.cpp

@@ -947,22 +947,25 @@ int collide_prop_weapon(obj_pair* pair)
 /**
  * Upper limit estimate ship speed at end of time
  */
-static float estimate_ship_speed_upper_limit( object *ship, float time ) 
+static float estimate_ship_speed_upper_limit( object *objp, float time )
 {
 	float exponent;
 	float delta_v;
 	float factor;
 
-	delta_v = Ship_info[Ships[ship->instance].ship_info_index].max_vel.xyz.z - ship->phys_info.speed;
+	if (objp->type != OBJ_SHIP)
+		return objp->phys_info.speed;
+
+	delta_v = Ship_info[Ships[objp->instance].ship_info_index].max_vel.xyz.z - objp->phys_info.speed;
 	
-	if (ship->phys_info.forward_accel_time_const == 0) {
-		return ship->phys_info.speed;
+	if (objp->phys_info.forward_accel_time_const == 0) {
+		return objp->phys_info.speed;
 	}
 	
-	exponent = time / ship->phys_info.forward_accel_time_const;
+	exponent = time / objp->phys_info.forward_accel_time_const;
 
 	factor = 1.0f - (float)exp( -exponent );
-	return ship->phys_info.speed + factor*delta_v;
+	return objp->phys_info.speed + factor*delta_v;
 }
 
 // maximum error allowed in detecting collisions between laser and big ship inside the radius

code/object/objcollide.cpp

@@ -1100,7 +1100,7 @@ void obj_collide_pair(object *A, object *B)
                 }
 
                 // for nonplayer ships, only create collision pair if close enough
-                if ( (B->parent >= 0) && !((Objects[B->parent].signature == B->parent_sig) && (Objects[B->parent].flags[Object::Object_Flags::Player_ship])) && (vm_vec_dist(&B->pos, &A->pos) < (4.0f*A->radius + 200.0f)) ) {
+                if ( (B->parent >= 0) && ((Objects[B->parent].signature != B->parent_sig) || !(Objects[B->parent].flags[Object::Object_Flags::Player_ship])) && (vm_vec_dist_squared(&B->pos, &A->pos) < (4.0f*A->radius + 200.0f) * (4.0f*A->radius + 200.0f)) ) {
                     collision_info->next_check_time = -1;
                     return;
                 }

code/object/object.cpp

@@ -142,10 +142,13 @@ const char *Object_type_names[MAX_OBJECT_TYPES] = {
 	"Asteroid",
 	"Jump Node",
 	"Beam",
-	"Raw Pof"
+	"Raw Pof",
+	"Prop"
 //XSTR:ON
 };
 
+static_assert(MAX_OBJECT_TYPES == OBJ_PROP + 1, "Object_type_names needs an entry for every object type!");
+
 obj_flag_name Object_flag_names[] = {
     { Object::Object_Flags::Invulnerable,			"invulnerable",						},
 	{ Object::Object_Flags::Protected,				"protect-ship",						},
@@ -1412,7 +1415,9 @@ void obj_move_all_post(object *objp, float frametime)
 
 			//Check for changing team colors
 			ship* shipp = &Ships[objp->instance];
-			if (Ship_info[shipp->ship_info_index].uses_team_colors && stricmp(shipp->secondary_team_name.c_str(), "none") != 0) {
+			// team_change_time is nonzero only while a fade is in progress (see sexp_change_team_color),
+			// so use that as the initial short-circuit check before the string match
+			if (shipp->team_change_time != 0 && Ship_info[shipp->ship_info_index].uses_team_colors && stricmp(shipp->secondary_team_name.c_str(), "none") != 0) {
 				if (f2fl(Missiontime) * 1000 > f2fl(shipp->team_change_timestamp) * 1000 + shipp->team_change_time) {
 					shipp->team_name = shipp->secondary_team_name;
 					shipp->team_change_timestamp = 0;
@@ -1702,7 +1707,7 @@ void obj_move_all(float frametime)
 		obj_move_all_post(objp, frametime);
 
 		// Equipment script processing
-		if (objp->type == OBJ_SHIP) {
+		if (objp->type == OBJ_SHIP && scripting::hooks::OnWeaponEquipped->isActive()) {
 			ship* shipp = &Ships[objp->instance];
 			object* target;
 
@@ -1713,13 +1718,11 @@ void obj_move_all(float frametime)
 			if (objp == Player_obj && Player_ai->target_objnum != -1)
 				target = &Objects[Player_ai->target_objnum];
 
-			if (scripting::hooks::OnWeaponEquipped->isActive()) {
-				scripting::hooks::OnWeaponEquipped->run(scripting::hooks::WeaponEquippedConditions{ shipp, target },
-					scripting::hook_param_list(
-						scripting::hook_param("User", 'o', objp),
-						scripting::hook_param("Target", 'o', target)
-					));
-			}
+			scripting::hooks::OnWeaponEquipped->run(scripting::hooks::WeaponEquippedConditions{ shipp, target },
+				scripting::hook_param_list(
+					scripting::hook_param("User", 'o', objp),
+					scripting::hook_param("Target", 'o', target)
+				));
 		}
 	}
 

code/object/object.h

@@ -54,7 +54,7 @@
 #define OBJ_PROP            17  // A prop object like a landscape. Similar to ships but is entirely non-interactable with the exception of collisions
 
 //Make sure to change Object_type_names in Object.c when adding another type!
-#define MAX_OBJECT_TYPES	17
+#define MAX_OBJECT_TYPES	18
 
 #define UNUSED_OBJNUM		(-MAX_OBJECTS*2)	//	Newer systems use this instead of -1 for invalid object.
 
@@ -250,7 +250,7 @@ extern object *Player_obj;	// Which object is the player. Has to be valid.
 // given it's pointer.  This way, we can replace it with a macro
 // to check that the pointer is valid for debugging.
 // This code will break in 64 bit builds when we have more than 2^31 objects but that will probably never happen
-#define OBJ_INDEX(objp) static_cast<int>(objp-Objects)
+#define OBJ_INDEX(objp) (static_cast<int>((objp)-Objects))
 
 /*
  *		FUNCTIONS

code/parse/parselo.cpp

@@ -2988,6 +2988,79 @@ int stuff_long(long *l, bool optional)
 	return retval;
 }
 
+//	Stuff an unsigned 64-bit value pointed at by Mp.
+//	Advances past integer characters.
+int stuff_uint64(std::uint64_t *l, bool optional)
+{
+	char *str_start = Mp;
+
+	// since strtoull ignores white space anyway, might as well make it explicit
+	ignore_white_space();
+
+	// this is a bit cumbersome
+	size_t span;
+	if (*Mp == '+')
+	{
+		span = strspn(Mp + 1, "0123456789");
+
+		// account for the sign symbol, but not if it's the only valid character
+		if (span > 0)
+			++span;
+	}
+	else
+		span = strspn(Mp, "0123456789");
+
+	// don't call strtoull unless we found digits, because it will happily parse
+	// (and wrap around) a negative number
+	std::uint64_t result = (span > 0) ? strtoull(Mp, nullptr, 10) : 0;
+	bool success = false, comma = false;
+	int retval = 0;
+
+	// no number found?
+	if (span == 0)
+	{
+		if (!optional)
+			error_display(1, "Expected unsigned integer, found [%.32s].\n", next_tokens());
+	}
+	else
+	{
+		*l = result;
+		success = true;
+	}
+
+	if (success)
+		Mp += span;
+
+	// if an unexpected character is part of the number, warn about it
+	if (success && unexpected_numeric_char(*Mp))
+	{
+		error_display(0, "Expected unsigned integer, found [%.32s].\n", next_tokens(true));
+		// Rather than back up to str_start, do what retail did and continue
+		// merrily parsing along at the next character.  (Optional numbers
+		// will still back up to str_start - c.f. a few lines down.)
+		if (optional)
+			success = false;
+	}
+
+	if (check_first_non_grayspace_char(Mp, ',', &Mp))
+		comma = true;
+
+	if (optional && !success)
+		Mp = str_start;
+
+	if (success)
+	{
+		retval = 2;
+		diag_printf("Stuffed uint64: " UINT64_T_ARG "\n", *l);
+	}
+	else if (optional)
+		retval = comma ? 1 : 0;
+	else
+		skip_token();
+
+	return retval;
+}
+
 int stuff_float_optional(float *f)
 {
 	return stuff_float(f, true);

code/parse/parselo.h

@@ -164,6 +164,7 @@ extern bool check_first_non_grayspace_char(const char *str, char ch_to_look_for,
 extern int stuff_float(float *f, bool optional = false);
 extern int stuff_int(int *i, bool optional = false);
 extern int stuff_long(long *l, bool optional = false);
+extern int stuff_uint64(std::uint64_t *l, bool optional = false);
 extern void stuff_ubyte(ubyte *i);
 extern int stuff_int_optional(int *i);
 extern int stuff_float_optional(float *f);
@@ -246,17 +247,14 @@ void parse_string_flag_list_special(Flagset& dest, const special_flag_def_list_n
 }
 
 template<class T>
-void stuff_flagset(T *dest) {
-    long l = 0;
-    stuff_long(&l);
+void stuff_flagset(T *dest)
+{
+    std::uint64_t val = 0;
+    stuff_uint64(&val);
 
-	if (l < 0) {
-		error_display(0, "Expected flagset value but got negative value %lu!\n", l);
-		l = 0;
-	}
-    dest->from_u64((std::uint64_t) l);
+    dest->from_u64(val);
 
-    diag_printf("Stuffed flagset: %" PRIu64 "\n", dest->to_u64());
+    diag_printf("Stuffed flagset: " UINT64_T_ARG "\n", dest->to_u64());
 }
 
 extern size_t stuff_int_list(int *ilp, size_t max_ints, ParseLookupType lookup_type = ParseLookupType::RAW_INTEGER_TYPE, bool warn_on_lookup_failure = true);

code/parse/sexp.cpp

@@ -1989,17 +1989,18 @@ int query_sexp_args_count(int node, bool only_valid_args = false)
 	return count;
 }
 
+enum class ArgCountCheck { CORRECT, INCORRECT_BENIGN, INCORRECT_FATAL };
 /**
  * Needed to fix bug with sexps like send-message list which have arguments that need to be supplied as a block
  * 
  * @return whether the number of arguments for the supplied operation is correct
  */
-static bool check_operator_argument_count(int count, int op_index)
+static ArgCountCheck check_operator_argument_count(int count, int op_index)
 {
 	Assertion(op_index >= 0 && op_index < sz2i(Operators.size()), "op_index is out of range!");
 
 	if (count < Operators[op_index].min || count > Operators[op_index].max)
-		return false;
+		return ArgCountCheck::INCORRECT_FATAL;
 
 	int op_const = Operators[op_index].value;
 
@@ -2011,9 +2012,9 @@ static bool check_operator_argument_count(int count, int op_index)
 
 	if (op_const == OP_SEND_MESSAGE_LIST || op_const == OP_SEND_MESSAGE_CHAIN)
 		if (count % 4 != 0)
-			return false;
+			return ArgCountCheck::INCORRECT_BENIGN;		// historically, this check didn't work at all, and sexps gracefully recovered at runtime
 
-	return true;
+	return ArgCountCheck::CORRECT;
 }
 
 // helper functions for check_container_value_data_type()
@@ -2159,6 +2160,7 @@ int check_sexp_syntax(int node, int desired_return_type, int recursive, int *bad
 	int var_index = -1;
 	size_t st;
 	const sexp_container *p_container = nullptr; // for SEXPs that take container name as arg
+	int deferred_error = SEXP_CHECK_NO_ERROR, deferred_bad_node = -1;	// for recoverable errors, so that the rest of the tree is still checked
 
 	Assertion(node >= 0 && node < Num_sexp_nodes, "Node %d must be a valid SEXP node!", node);
 	Assertion(Sexp_nodes[node].type != SEXP_NOT_USED, "Node %d must be in use!", node);
@@ -2207,8 +2209,15 @@ int check_sexp_syntax(int node, int desired_return_type, int recursive, int *bad
 
 	count = query_sexp_args_count(op_node);
 
-	if (!check_operator_argument_count(sz2i(count), op_index))
-		return SEXP_CHECK_BAD_ARG_COUNT;  // incorrect number of arguments
+	auto arg_count_result = check_operator_argument_count(sz2i(count), op_index);
+	if (arg_count_result == ArgCountCheck::INCORRECT_FATAL)
+		return SEXP_CHECK_BAD_ARG_COUNT;	// incorrect number of arguments
+	else if (arg_count_result == ArgCountCheck::INCORRECT_BENIGN)
+	{
+		// incorrect, but defer it and continue checking
+		deferred_error = SEXP_CHECK_BAD_ARG_COUNT_BENIGN;
+		deferred_bad_node = op_node;
+	}
 
 	node = Sexp_nodes[op_node].rest;
 	while (node != -1) {
@@ -2227,7 +2236,7 @@ int check_sexp_syntax(int node, int desired_return_type, int recursive, int *bad
 			// thing.  (i.e. in the case of a cond statement, the conditional will fall into this if
 			// statement.  MORE TO DO HERE!!!!
 			if (Sexp_nodes[i].subtype == SEXP_ATOM_LIST)
-				return 0;
+				break;
 
 			int op2_index = get_operator_index(i);
 			int op2_const = SCP_vector_inbounds(Operators, op2_index) ? Operators[op2_index].value : OP_NOT_AN_OP;
@@ -2242,10 +2251,19 @@ int check_sexp_syntax(int node, int desired_return_type, int recursive, int *bad
 				}
 
 				if ((z = check_sexp_syntax(i, (int)opr, recursive, bad_node)) != 0) {
-					return z;
+					if (!sexp_recoverable_error(z))
+						return z;
+					// defer recoverable errors so that the rest of the tree is still checked
+					if (deferred_error == SEXP_CHECK_NO_ERROR) {
+						deferred_error = z;
+						deferred_bad_node = bad_node ? *bad_node : -1;
+					}
 				}
 			}
 
+		} else if (node_subtype == SEXP_ATOM_OPERATOR) {
+			return SEXP_CHECK_DATA_EXPECTED;	// operators should not be found here
+
 		} else if (node_subtype == SEXP_ATOM_NUMBER) {
 			node_return_type = OPR_POSITIVE;
 			auto ptr = CTEXT(node);
@@ -2376,9 +2394,8 @@ int check_sexp_syntax(int node, int desired_return_type, int recursive, int *bad
 			continue;
 
 		} else {
-			UNREACHABLE("SEXP subtype is %d when it should be SEXP_ATOM_LIST, SEXP_ATOM_NUMBER, SEXP_ATOM_STRING, "
-						"SEXP_ATOM_CONTAINER_NAME, or "
-						"SEXP_ATOM_CONTAINER_DATA!",
+			UNREACHABLE("SEXP subtype is %d when it should be SEXP_ATOM_LIST, SEXP_ATOM_OPERATOR, SEXP_ATOM_NUMBER, "
+						"SEXP_ATOM_STRING, SEXP_ATOM_CONTAINER_NAME, or SEXP_ATOM_CONTAINER_DATA!",
 				node_subtype);
 		}
 
@@ -3153,7 +3170,13 @@ int check_sexp_syntax(int node, int desired_return_type, int recursive, int *bad
 				// we should check the syntax of the actual goal!!!!
 				z = Sexp_nodes[node].first;
 				if ((z = check_sexp_syntax(z, OPR_AI_GOAL, recursive, bad_node)) != 0){
-					return z;
+					if (!sexp_recoverable_error(z))
+						return z;
+					// defer recoverable errors so that the rest of the tree is still checked
+					if (deferred_error == SEXP_CHECK_NO_ERROR) {
+						deferred_error = z;
+						deferred_bad_node = bad_node ? *bad_node : -1;
+					}
 				}
 
 				if (Fred_running) {
@@ -4209,6 +4232,14 @@ int check_sexp_syntax(int node, int desired_return_type, int recursive, int *bad
 		argnum++;
 	}
 
+	// now that the rest of the tree has been checked, report any recoverable error that was noted along the way
+	if (deferred_error != SEXP_CHECK_NO_ERROR)
+	{
+		if (bad_node)
+			*bad_node = deferred_bad_node;
+		return deferred_error;
+	}
+
 	return 0;
 }
 
@@ -5813,7 +5844,15 @@ const ship_registry_entry *eval_ship(int node)
 			return eval_ship(arg_node);
 	}
 
-	auto ship_it = Ship_registry_map.find(CTEXT(node));
+	// look up the ship in the ship registry
+	auto ship_name = CTEXT(node);
+	auto ship_it = Ship_registry_map.find(ship_name);
+	if (ship_it == Ship_registry_map.end())
+	{
+		SCP_string legacy_hashed;
+		if (wing_bash_legacy_hashed_ship_name(legacy_hashed, ship_name))
+			ship_it = Ship_registry_map.find(legacy_hashed);
+	}
 	if (ship_it != Ship_registry_map.end())
 	{
 		// cache the value if it can't change later
@@ -10341,6 +10380,11 @@ int sexp_percent_ships_arrive_depart_destroy_disarm_disable_scan(int n, int what
 		}
 	}
 
+	// if there is nothing to check, the percentage is meaningless; this can happen if, for example, a wing
+	// arrives from a docking bay and its mothership is destroyed before the wing has a chance to arrive
+	if ( total <= 0 )
+		return SEXP_FALSE;
+
 	// now, look at the percentage
 	if ( ((count * 100) / total) >= percent )
 		return SEXP_KNOWN_TRUE;
@@ -12808,7 +12852,7 @@ int sexp_is_iff_or_species(int n, bool iff)
 					// ship is in the EXITED state but probably in the process of exploding
 					else if (oswpt.has_shipp())
 					{
-						UNREACHABLE("With the addition of the ShipStatus::DEATH_ROLL state, this shouldn't happen");
+						Assertion(false, "With the addition of the ShipStatus::DEATH_ROLL state, this shouldn't happen");
 						return SEXP_KNOWN_FALSE;
 					}
 					// ship has vanished
@@ -13966,8 +14010,9 @@ void sexp_set_player_target(int node)
 			new_subsys = ship_get_subsys(shipp, subsys_name);
 		}
 	}
-    set_target_objnum(Player_ai, objnum);
-    set_targeted_subsys(Player_ai, new_subsys, objnum);
+	set_target_objnum(Player_ai, objnum);
+	set_targeted_subsys(Player_ai, new_subsys, new_subsys ? objnum : -1);
+	shipp->last_targeted_subobject[Player_num] = new_subsys;
 }
 
 // Luytenky
@@ -35347,11 +35392,13 @@ bool sexp_recoverable_error(int num)
 		// but the mission will run without crashing.
 		case SEXP_CHECK_AMBIGUOUS_EVENT_NAME:
 		case SEXP_CHECK_AMBIGUOUS_GOAL_NAME:
+			return true;
 
 		// Having an invalid gauge in FSO won't hurt,
 		// as all places which call hud_get_gauge() or hud_get_custom_gauge() check its return value for NULL.
 		case SEXP_CHECK_INVALID_CUSTOM_HUD_GAUGE:
 		case SEXP_CHECK_INVALID_ANY_HUD_GAUGE:
+			return true;
 
 		// Trying to set an invalid sound environment has no effect, and all sound enviroments are invalid if EFX is disabled.
 		// Invalid sound environment options are simiarly harmless.
@@ -35359,6 +35406,10 @@ bool sexp_recoverable_error(int num)
 		case SEXP_CHECK_INVALID_SOUND_ENVIRONMENT_OPTION:
 			return true;
 
+		// Certain argument counts historically weren't checked properly, but the runtime code could still recover
+		case SEXP_CHECK_BAD_ARG_COUNT_BENIGN:
+			return true;
+
 		// most errors will halt mission loading
 		default:
 			return false;
@@ -35374,6 +35425,9 @@ const char *sexp_error_message(int num)
 		case SEXP_CHECK_OP_EXPECTED:
 			return "Operator expected instead of data";
 
+		case SEXP_CHECK_DATA_EXPECTED:
+			return "Data expected instead of operator";
+
 		case SEXP_CHECK_UNKNOWN_OP:
 			return "Unrecognized operator";
 
@@ -35381,6 +35435,7 @@ const char *sexp_error_message(int num)
 			return "Argument type mismatch";
 
 		case SEXP_CHECK_BAD_ARG_COUNT:
+		case SEXP_CHECK_BAD_ARG_COUNT_BENIGN:
 			return "Argument count is illegal";
 
 		case SEXP_CHECK_UNKNOWN_TYPE:

code/parse/sexp.h

@@ -1203,9 +1203,11 @@ enum sexp_error_check
 
 	SEXP_CHECK_NONOP_ARGS,              // non-operator has arguments
 	SEXP_CHECK_OP_EXPECTED,             // operator expected, but found data instead
+	SEXP_CHECK_DATA_EXPECTED,           // data expected, but found operator instead
 	SEXP_CHECK_UNKNOWN_OP,              // unrecognized operator
 	SEXP_CHECK_TYPE_MISMATCH,           // return type or data type mismatch
-	SEXP_CHECK_BAD_ARG_COUNT,           // argument count in incorrect
+	SEXP_CHECK_BAD_ARG_COUNT,           // argument count is incorrect
+	SEXP_CHECK_BAD_ARG_COUNT_BENIGN,    // ditto, but don't prevent the mission from loading
 	SEXP_CHECK_UNKNOWN_TYPE,            // unrecognized return type of data type
 
 	SEXP_CHECK_INVALID_NUM = 101,       // number is not valid

code/parse/sexp/LuaAISEXP.cpp

@@ -154,13 +154,6 @@ void LuaAISEXP::parseTable() {
 				// Now get a list of all lua categories to add. Meow.
 				SCP_vector<SCP_string> lua_cat_list = ai_lua_get_general_order_categories();
 
-				// If we have too many categories then we have an oopsie!
-				if ((int)lua_cat_list.size() > (MAX_MENU_ITEMS - NUM_COMM_ORDER_TYPES)) {
-					Warning(LOCATION, "Too many defined Lua General Order Categories! Setting order to first available: %s", lua_cat_list[0].c_str());
-
-					order.category = lua_cat_list[0];
-				}
-
 				// General orders explicitely do not show up in FRED as sexps because they
 				// they are only meant for the comms board
 				_return_type = OPR_NONE;

code/particle/ParticleManager.cpp

@@ -243,6 +243,15 @@ namespace internal {
 		SCP_vector<int> handles;
 
 		for (auto const &name: bitmap_strings) {
+			// FRED never renders or pages in particle effects, 
+			// so skip the (potentially expensive) animation loading here.  
+			// Still record placeholder handle so the effect stays "defined" and 
+			// can be resolved by name from the weapons/ships tables without spurious errors.
+			if (Fred_running) {
+				handles.push_back(0);
+				continue;
+			}
+
 			auto handle = bm_load_animation(name.c_str());
 			if (handle >= 0) {
 				handles.push_back(handle);

code/playerman/playercontrol.cpp

@@ -417,6 +417,15 @@ void do_view_track_target()
 	chase_slew_angles.h = forward_angles.h - view_angles.h;
 	chase_slew_angles.p = -(forward_angles.p - view_angles.p);
 
+	// Do over-the-top correction.
+	// Headings are extracted with atan2, so each one lies in (-PI, PI] and their difference can be
+	// nearly a full circle in either direction.  Without wrapping it back into (-PI, PI], a target just
+	// past the left shoulder reads as being almost all the way around to the right instead.
+	if (chase_slew_angles.h > PI)
+		chase_slew_angles.h -= PI2;
+	else if (chase_slew_angles.h < -PI)
+		chase_slew_angles.h += PI2;
+
 	// the gimbal limits of the player's virtual neck.
 	// These nested ifs prevent the player from looking up and 
 	// down beyond 90 degree angles.
@@ -1420,7 +1429,6 @@ void player_restore_target_and_weapon_link_prefs()
 {
 	ship_info *player_sip;
 	player_sip = &Ship_info[Player_ship->ship_info_index];
-	polymodel *pm = model_get(player_sip->model_num);
 
 	//	Don't restores the save flags in training, as we must ensure certain things are off, such as speed matching.
 	if ( !(The_mission.game_type & MISSION_TYPE_TRAINING )) {
@@ -1433,7 +1441,9 @@ void player_restore_target_and_weapon_link_prefs()
 		}
 	}
 
-	if ( Player->flags & PLAYER_FLAGS_LINK_SECONDARY && (pm->n_missiles > 0 && pm->missile_banks[0].num_slots > 1) ) {
+	// restore the dual fire preference regardless of whether the current bank can use it;
+	// the flag is ignored rather than cleared for banks that can't
+	if ( Player->flags & PLAYER_FLAGS_LINK_SECONDARY ) {
 		Player_ship->flags.set(Ship::Ship_Flags::Secondary_dual_fire);
 	}
 }
@@ -2079,6 +2089,10 @@ void player_generate_death_message(player *player_p)
 			sprintf(msg, XSTR( "%s was killed by a collision with an asteroid", 98), player_p->callsign);
 			break;
 
+		case OBJ_PROP:
+			sprintf(msg, XSTR( "%s was killed by a collision with an object", -1), player_p->callsign);
+			break;
+
 		case OBJ_BEAM:
 			if (strlen(player_p->killer_parent_name) <= 0)
 			{

code/prop/prop.cpp

@@ -473,9 +473,8 @@ int prop_create(const matrix* orient, const vec3d* pos, int prop_type, const cha
 		Error(LOCATION, "Cannot create prop %s; pof file is not valid", pip->name.c_str());
 		return -1;
 	}
-	if (pip->model_num == -1) {
-		pip->model_num = model_load(pip->pof_file.c_str());
-	}
+
+	pip->model_num = model_load(pip->pof_file.c_str());
 
 	polymodel* pm = model_get(pip->model_num);
 
@@ -613,9 +612,7 @@ static void prop_model_change(int n, int prop_type)
 	//polymodel_instance* pmi = model_get_instance(sp->model_instance_num);
 
 	// get new model
-	if (sip->model_num == -1) {
-		sip->model_num = model_load(sip->pof_file.c_str());
-	}
+	sip->model_num = model_load(sip->pof_file.c_str());
 
 	polymodel* pm = model_get(sip->model_num);
 	Objects[sp->objnum].radius = model_get_radius(pm->id);

code/radar/radarsetup.cpp

@@ -290,12 +290,12 @@ void radar_plot_object( object *objp )
 	vm_vec_sub(&tempv, &world_pos, &Player_obj->pos);
 	vm_vec_rotate(&pos, &tempv, &eye_orient);
 
-	// Apply range filter
-	dist = vm_vec_dist(&world_pos, &Player_obj->pos);
+	// Apply range filter (squared-distance cull first so the sqrt only runs for blips that survive)
 	max_radar_dist = Radar_ranges[HUD_config.rp_dist];
-	if (dist > max_radar_dist) {
+	float dist_sq = vm_vec_mag_squared(&tempv);
+	if (dist_sq > max_radar_dist * max_radar_dist)
 		return;
-	}
+	dist = sqrtf(dist_sq);
 
 	// Mine range-based visibility: sensors_range is the outer detection envelope. The parser
 	// guarantees sensors_range >= targetable_range, so anything within targetable range is
@@ -572,7 +572,7 @@ RadarVisibility radar_is_visible( object *objp )
 		return NOT_VISIBLE;
 
 	vec3d pos, tempv;
-	float awacs_level, dist, max_radar_dist;
+	float awacs_level;
 	vec3d world_pos = objp->pos;
 
 	// get team-wide awacs level for the object if not ship
@@ -651,13 +651,12 @@ RadarVisibility radar_is_visible( object *objp )
 	vm_vec_sub(&tempv, &world_pos, &Player_obj->pos);
 	vm_vec_rotate(&pos, &tempv, &Player_obj->orient);
 
-	// Apply range filter
-	dist = vm_vec_dist(&world_pos, &Player_obj->pos);
-	max_radar_dist = Radar_ranges[HUD_config.rp_dist];
-	if (dist > max_radar_dist) {
+	// Apply range filter (squared-distance cull first so the sqrt only runs for blips that survive)
+	float max_radar_dist = Radar_ranges[HUD_config.rp_dist];
+	float dist_sq = vm_vec_mag_squared(&tempv);
+	if (dist_sq > max_radar_dist * max_radar_dist)
 		return NOT_VISIBLE;
-	}
-	
+
 	if (objp->type == OBJ_SHIP)
 	{
 		// ships specifically hidden from sensors

code/scpui/rocket_ui.cpp

@@ -13,6 +13,7 @@
 #include "scpui/rocket_ui.h"
 
 #include "cfile/cfile.h"
+#include "io/mouse.h"
 #include "mod_table/mod_table.h"
 #include "osapi/osapi.h"
 #include "scpui/IncludeNodeHandler.h"
@@ -59,6 +60,26 @@ vec2d render_offset    = {0.f, 0.f};
 
 bool debugger_initialized = false;
 
+/**
+ * @brief Retrieves the current cursor position in render-target pixels
+ *
+ * @details SDL reports event coordinates in window pixels, but a libRocket context is sized from
+ * gr_screen.max_w/max_h and renders through gr_set_2d_matrix, which is also render space. Rather
+ * than duplicating the conversion here we let mouse_get_real_pos do it.
+ *
+ * Note that this is the cursor position as of right now rather than the one carried by the event,
+ * so if several motion events are queued in a single os_poll libRocket sees the latest position
+ * for all of them. That is what it wants for hover state anyway.
+ */
+vec2d get_render_space_mouse_pos()
+{
+	int x = 0;
+	int y = 0;
+	mouse_get_real_pos(&x, &y);
+
+	return {i2fl(x), i2fl(y)};
+}
+
 bool transform_to_rocket(vec2d& in)
 {
 	in = {in.x - render_offset.x, in.y - render_offset.y};
@@ -145,7 +166,7 @@ bool mouse_motion_handler(const SDL_Event& evt)
 		return false;
 	}
 
-	vec2d evt_pos = {(float)evt.motion.x, (float)evt.motion.y};
+	vec2d evt_pos = get_render_space_mouse_pos();
 	if (!transform_to_rocket(evt_pos)) {
 		// Out of bounds
 		return false;
@@ -168,7 +189,7 @@ bool mouse_button_handler(const SDL_Event& evt)
 		return false;
 	}
 
-	vec2d evt_pos = {(float)evt.button.x, (float)evt.button.y};
+	vec2d evt_pos = get_render_space_mouse_pos();
 	if (!transform_to_rocket(evt_pos)) {
 		// Out of bounds
 		return false;

code/scripting/api/libs/mission.cpp

@@ -343,7 +343,7 @@ ADE_INDEXER(l_Mission_Comm_Items,
 	// convert from lua index
 	idx--;
 
-	if ((idx < 0) || idx >= MAX_MENU_ITEMS)
+	if (idx < 0)
 		return ade_set_args(L, "o", l_Comm_Item.Set(-1));
 
 	return ade_set_args(L, "o", l_Comm_Item.Set(idx));
@@ -356,7 +356,7 @@ ADE_FUNC(__len,
 	"number",
 	"Number of comm orders in the mission. 0 if comm menu is closed")
 {
-	return ade_set_args(L, "i", Num_menu_items);
+	return ade_set_args(L, "i", sz2i(MsgItems.size()));
 }
 
 //****SUBLIBRARY: Mission/Debris

code/scripting/api/objs/comm_order.cpp

@@ -17,8 +17,11 @@ ADE_VIRTVAR(Name, l_Comm_Item, nullptr, "The name of the comm item", "string", "
 	if (ADE_SETTING_VAR) {
 		LuaError(L, "This property is read only.");
 	}
-
-	return ade_set_args(L, "s", MsgItems[current].text.c_str());
+	SCP_string message = "Invalid comm item!";
+	if (MsgItems.in_bounds(current)) {
+		message = MsgItems[current].text;
+	}
+	return ade_set_args(L, "s", message.c_str());
 }
 
 ADE_VIRTVAR(Active, l_Comm_Item, nullptr, "Whether or not the item is active", "boolean", "The active status")
@@ -32,7 +35,7 @@ ADE_VIRTVAR(Active, l_Comm_Item, nullptr, "Whether or not the item is active", "
 		LuaError(L, "This property is read only.");
 	}
 
-	if (MsgItems[current].active > 0) {
+	if (MsgItems.in_bounds(current) && MsgItems[current].active > 0) {
 		return ADE_RETURN_TRUE;
 	}
 
@@ -60,7 +63,7 @@ ADE_FUNC(selectItem, l_Comm_Item, nullptr, "Selects the item and either proceeds
 	if (!ade_get_args(L, "o", l_Comm_Item.Get(&current)))
 		return ADE_RETURN_FALSE;
 
-	if (current < 0 || current >= Num_menu_items) {
+	if (!MsgItems.in_bounds(current)) {
 		LuaError(L, "Lua tried to select squad message that is not valid!");
 		return ADE_RETURN_FALSE;
 	}
@@ -77,7 +80,7 @@ ADE_FUNC(isValid, l_Comm_Item, nullptr, "Detect if the handle is valid", "boolea
 	if (!ade_get_args(L, "o", l_Comm_Item.Get(&current)))
 		return ADE_RETURN_FALSE;
 
-	return ade_set_args(L, "b", (current >= 0) && (current < Num_menu_items));
+	return ade_set_args(L, "b", MsgItems.in_bounds(current));
 }
 
 } // namespace scripting::api

code/scripting/api/objs/order.cpp

@@ -445,7 +445,7 @@ ADE_VIRTVAR(TargetSubsystem, l_Order, "subsystem", "Target subsystem of the orde
 				set_targeted_subsys(aip, newh->ss, OBJ_INDEX(ss_objp));
 			}
 			if (aip == Player_ai) {
-				Ships[newh->ss->parent_objnum].last_targeted_subobject[Player_num] = newh->ss;
+				ss_shipp->last_targeted_subobject[Player_num] = newh->ss;
 			}
 		}
 	}

code/scripting/api/objs/parse_object.cpp

@@ -848,7 +848,7 @@ ADE_VIRTVAR(SecondaryBanks, l_ParseSubsystem, nullptr, "The overridden secondary
 
 	luacpp::LuaTable tbl = luacpp::LuaTable::create(L);
 	auto po              = poh->getSubsys();
-	for (int i = 0; i < MAX_SHIP_PRIMARY_BANKS; ++i) {
+	for (int i = 0; i < MAX_SHIP_SECONDARY_BANKS; ++i) {
 		if (po->secondary_banks[i] == -1) {
 			break;
 		}
@@ -886,7 +886,7 @@ ADE_VIRTVAR(SecondaryAmmo, l_ParseSubsystem, nullptr, "The overridden secondary
 
 	luacpp::LuaTable tbl = luacpp::LuaTable::create(L);
 	auto po              = poh->getSubsys();
-	for (int i = 0; i < MAX_SHIP_PRIMARY_BANKS; ++i) {
+	for (int i = 0; i < MAX_SHIP_SECONDARY_BANKS; ++i) {
 		if (po->secondary_banks[i] == -1) {
 			break;
 		}

code/scripting/api/objs/ship.cpp

@@ -784,20 +784,21 @@ ADE_VIRTVAR(Target, l_Ship, "object", "Target of ship. Value may also be a deriv
 		return ade_set_error(L, "o", l_Object.Set(object_h()));
 
 	if(ADE_SETTING_VAR && !(newh && aip->target_signature == newh->sig)) {
-		// we have a different target, or are clearng the target
+		// we have a different target, or are clearing the target
 		if(newh && newh->isValid())	{
-			aip->target_objnum = newh->objnum;
-			aip->target_signature = newh->sig;
-			aip->target_time = 0.0f;
-			set_targeted_subsys(aip, nullptr, -1);
+			aip->ok_to_target_timestamp = timestamp(0);
+			set_target_objnum(aip, newh->objnum);
+
+			if (aip == Player_ai) {
+				// prevent hud_target_change_check() from restoring the wrong targeted subsystem
+				if (newh->objp()->type == OBJ_SHIP)
+					Ships[newh->objp()->instance].last_targeted_subobject[Player_num] = nullptr;
 
-			if (aip == Player_ai)
 				hud_shield_hit_reset(newh->objp());
+			}
 		} else if (lua_isnil(L, 2)) {
-			aip->target_objnum = -1;
-			aip->target_signature = -1;
-			aip->target_time = 0.0f;
-			set_targeted_subsys(aip, nullptr, -1);
+			aip->ok_to_target_timestamp = timestamp(0);
+			set_target_objnum(aip, -1);
 		}
 	}
 
@@ -825,25 +826,21 @@ ADE_VIRTVAR(TargetSubsystem, l_Ship, "subsystem", "Target subsystem of ship.", "
 	{
 		if(newh && newh->isValid())
 		{
-			if (aip == Player_ai) {
-				if (aip->target_signature != newh->objh.sig)
-					hud_shield_hit_reset(newh->objh.objp());
+			// this must be checked before the signature is updated in set_target_objnum
+			if (aip == Player_ai && aip->target_signature != newh->objh.sig)
+				hud_shield_hit_reset(newh->objh.objp());
 
+			aip->ok_to_target_timestamp = timestamp(0);
+			set_target_objnum(aip, newh->objh.objnum);
+			set_targeted_subsys(aip, newh->ss, newh->objh.objnum);
+
+			if (aip == Player_ai)
 				Ships[Objects[newh->ss->parent_objnum].instance].last_targeted_subobject[Player_num] = newh->ss;
-			}
-
-			aip->target_objnum = newh->objh.objnum;
-			aip->target_signature = newh->objh.sig;
-			aip->target_time = 0.0f;
-			set_targeted_subsys(aip, newh->ss, aip->target_objnum);
 		}
 		else
 		{
-			aip->target_objnum = -1;
-			aip->target_signature = -1;
-			aip->target_time = 0.0f;
-
-			set_targeted_subsys(aip, NULL, -1);
+			aip->ok_to_target_timestamp = timestamp(0);
+			set_target_objnum(aip, -1);
 		}
 	}
 

code/scripting/api/objs/ship_bank.cpp

@@ -55,11 +55,12 @@ ADE_INDEXER(l_WeaponBankType, "number Index", "Array of weapon banks", "weaponba
 			idx--; //Lua->FS2
 
 			if(ADE_SETTING_VAR && newbank && newbank->isValid()) {
-				sb->sw->primary_bank_weapons[idx] = newbank->typeh.sw->primary_bank_weapons[idx];
-				sb->sw->next_primary_fire_stamp[idx] = timestamp(0);
-				sb->sw->primary_bank_ammo[idx] = newbank->typeh.sw->primary_bank_ammo[idx];
-				sb->sw->primary_bank_start_ammo[idx] = newbank->typeh.sw->primary_bank_start_ammo[idx];
-				sb->sw->primary_bank_capacity[idx] = newbank->typeh.sw->primary_bank_capacity[idx];
+				sb->sw->secondary_bank_weapons[idx] = newbank->typeh.sw->secondary_bank_weapons[idx];
+				sb->sw->next_secondary_fire_stamp[idx] = timestamp(0);
+				sb->sw->secondary_bank_ammo[idx] = newbank->typeh.sw->secondary_bank_ammo[idx];
+				sb->sw->secondary_bank_start_ammo[idx] = newbank->typeh.sw->secondary_bank_start_ammo[idx];
+				sb->sw->secondary_bank_capacity[idx] = newbank->typeh.sw->secondary_bank_capacity[idx];
+				sb->sw->secondary_bank_rearm_time[idx] = timestamp(0);
 			}
 			break;
 		case SWH_TERTIARY:

code/ship/ship.cpp

@@ -238,6 +238,15 @@ int ship_registry_get_index(const char *name)
 	if (ship_it != Ship_registry_map.end())
 		return ship_it->second;
 
+	// also search for ship names hashed using the legacy format
+	SCP_string legacy_hashed;
+	if (wing_bash_legacy_hashed_ship_name(legacy_hashed, name))
+	{
+		ship_it = Ship_registry_map.find(legacy_hashed);
+		if (ship_it != Ship_registry_map.end())
+			return ship_it->second;
+	}
+
 	return -1;
 }
 
@@ -247,6 +256,15 @@ int ship_registry_get_index(const SCP_string &name)
 	if (ship_it != Ship_registry_map.end())
 		return ship_it->second;
 
+	// also search for ship names hashed using the legacy format
+	SCP_string legacy_hashed;
+	if (wing_bash_legacy_hashed_ship_name(legacy_hashed, name.c_str()))
+	{
+		ship_it = Ship_registry_map.find(legacy_hashed);
+		if (ship_it != Ship_registry_map.end())
+			return ship_it->second;
+	}
+
 	return -1;
 }
 
@@ -267,18 +285,18 @@ bool ship_registry_exists(int index)
 
 const ship_registry_entry *ship_registry_get(const char *name)
 {
-	auto ship_it = Ship_registry_map.find(name);
-	if (ship_it != Ship_registry_map.end())
-		return &Ship_registry[ship_it->second];
+	auto idx = ship_registry_get_index(name);
+	if (idx >= 0)
+		return &Ship_registry[idx];
 
 	return nullptr;
 }
 
 const ship_registry_entry *ship_registry_get(const SCP_string &name)
 {
-	auto ship_it = Ship_registry_map.find(name);
-	if (ship_it != Ship_registry_map.end())
-		return &Ship_registry[ship_it->second];
+	auto idx = ship_registry_get_index(name);
+	if (idx >= 0)
+		return &Ship_registry[idx];
 
 	return nullptr;
 }
@@ -297,6 +315,27 @@ const ship_registry_entry *ship_registry_get(anchor_t anchor)
 	return ship_registry_get(anchor.value());
 }
 
+void ship_registry_rename(int entry_index, const char *new_name, bool erase_old_key)
+{
+	Assertion(Ship_registry.in_bounds(entry_index), "Invalid ship registry index %d passed to ship_registry_rename!", entry_index);
+	if (!Ship_registry.in_bounds(entry_index))
+		return;
+
+	auto entry = &Ship_registry[entry_index];
+
+	// the old key is sometimes useful for looking up the ship under its previous name;
+	// if not, remove it (provided it actually refers to this entry)
+	if (erase_old_key)
+	{
+		auto ship_it = Ship_registry_map.find(entry->name);
+		if (ship_it != Ship_registry_map.end() && ship_it->second == entry_index)
+			Ship_registry_map.erase(ship_it);
+	}
+
+	strcpy_s(entry->name, new_name);
+	Ship_registry_map[entry->name] = entry_index;
+}
+
 
 int	Num_engine_wash_types;
 int	Num_ship_subobj_types;
@@ -4722,7 +4761,7 @@ static void parse_ship_values(ship_info* sip, const bool is_template, const bool
 		float help_hull_val;
 		stuff_float(&help_hull_val);
 		if (help_hull_val > 0.0f && help_hull_val <= 1.0f) {
-			sip->ask_help_shield_percent = help_hull_val;
+			sip->ask_help_hull_percent = help_hull_val;
 		} else {
 			error_display(0,"Ask Help Hull Percent for ship class %s is %f. This value is not within range of 0-1.0."
 			              "Assuming default value of %f.", sip->name, help_hull_val, DEFAULT_ASK_HELP_HULL_PERCENT);
@@ -6914,6 +6953,25 @@ void ship_level_init()
 	Man_thruster_reset_timestamp = timestamp(0);
 }
 
+void ship_level_close()
+{
+	// at this point ships have all gone through ship_delete;
+	// clean up any ships that are still present so we don't
+	// leave any stray references behind (e.g. for sexps)
+	for (auto &ship_entry : Ship_registry)
+	{
+		if (ship_entry.status == ShipStatus::PRESENT || ship_entry.status == ShipStatus::DEATH_ROLL)
+		{
+			ship_entry.cleanup_mode = (ship_entry.status == ShipStatus::DEATH_ROLL) ? SHIP_DESTROYED : SHIP_PRESENT_AT_MISSION_END;
+			ship_entry.status = ShipStatus::EXITED;
+			ship_entry.objnum = -1;
+			ship_entry.shipnum = -1;
+		}
+	}
+
+	ship_close_cockpit_displays(Player_ship);
+}
+
 /**
  * Add a ship onto the exited ships list.
  *
@@ -8745,6 +8803,7 @@ void ship_close_cockpit_displays(ship* shipp)
 {
 	if (shipp && shipp->cockpit_model_instance >= 0) {
 		model_delete_instance(shipp->cockpit_model_instance);
+		shipp->cockpit_model_instance = -1;
 	}
 
 	for ( int i = 0; i < (int)Player_displays.size(); i++ ) {
@@ -14229,6 +14288,37 @@ static bool ship_fire_secondary_detonate(object *obj, ship_weapon *swp)
 
 extern void ai_maybe_announce_shockwave_weapon(object *firing_objp, int weapon_index);
 
+// Determines whether this secondary bank is currently capable of dual fire.  Note that the
+// Secondary_dual_fire flag can legitimately be set even when this returns false: the flag is
+// ignored rather than cleared so that the player's dual fire preference isn't lost when
+// cycling through banks or weapons.  All code that acts on the flag should check this too.
+bool ship_secondary_bank_can_dual_fire(const ship *shipp, int bank)
+{
+	if (bank < 0 || bank >= shipp->weapons.num_secondary_banks)
+		return false;
+
+	int weapon_class = shipp->weapons.secondary_bank_weapons[bank];
+	if (weapon_class < 0)
+		return false;
+
+	if (Weapon_info[weapon_class].wi_flags[Weapon::Info_Flags::No_doublefire])
+		return false;
+
+	if (shipp->objnum == OBJ_INDEX(Player_obj))
+	{
+		if (The_mission.ai_profile->flags[AI::Profile_Flags::Disable_player_secondary_doublefire])
+			return false;
+	}
+	else
+	{
+		if (The_mission.ai_profile->flags[AI::Profile_Flags::Disable_ai_secondary_doublefire])
+			return false;
+	}
+
+	polymodel *pm = model_get(Ship_info[shipp->ship_info_index].model_num);
+	return pm->missile_banks[bank].num_slots > 1;
+}
+
 //	Object *obj fires its secondary weapon, if it can.
 //	If its most recently fired weapon is a remotely detonatable weapon, detonate it.
 //	Returns number of weapons fired.  Note, for swarmers, returns 1 if it is allowed
@@ -14591,25 +14681,14 @@ int ship_fire_secondary( object *obj, int allow_swarm, bool rollback_shot )
 			goto done_secondary;
 		}
 
-		// Handle the optional disabling of dual fire
-		// dual fire/doublefire can be disabled for individual weapons, for players, or for AIs
-		// if any of these apply to the current weapon, unset the dual fire flag on the ship 
-		// then proceed as normal.
-		// This is only handled at firing time so dualfire isn't lost when cycling through weapons
-		if (shipp->flags[Ship_Flags::Secondary_dual_fire] &&
-			( wip->wi_flags[Weapon::Info_Flags::No_doublefire] || 
-			( The_mission.ai_profile->flags[AI::Profile_Flags::Disable_ai_secondary_doublefire] && 
-				shipp->objnum != OBJ_INDEX(Player_obj) ) ||
-			( The_mission.ai_profile->flags[AI::Profile_Flags::Disable_player_secondary_doublefire] &&
-				shipp->objnum == OBJ_INDEX(Player_obj) ))
-			) {
-			shipp->flags.remove(Ship_Flags::Secondary_dual_fire);
-		}
-
 		int start_slot, end_slot;
 		no_energy = shipp->weapon_energy < 2 * wip->energy_consumed; // whether there's enough energy for at least 1 shot was checked above
 
-		if ( shipp->flags[Ship_Flags::Secondary_dual_fire] && num_slots > 1) {
+		// Dual fire can be unavailable for this bank because it has only one firepoint, because
+		// the weapon disallows it, or because of an ai_profiles restriction.  In any of these
+		// cases the flag is ignored rather than cleared so the dual fire preference isn't lost
+		// when cycling through banks or weapons.
+		if ( shipp->flags[Ship_Flags::Secondary_dual_fire] && ship_secondary_bank_can_dual_fire(shipp, bank) ) {
 			start_slot = swp->secondary_next_slot[bank];
 			// AL 11-19-97: Ensure enough ammo remains when firing linked secondary weapons
 			if ( check_ammo && ((swp->secondary_bank_ammo[bank] < 2 && !no_ammo_needed) || no_energy) ) {
@@ -14618,9 +14697,6 @@ int ship_fire_secondary( object *obj, int allow_swarm, bool rollback_shot )
 				end_slot = start_slot+1;
 			}
 		} else {
-			// de-set the flag just in case dual-fire was set but couldn't be used
-			// because there's less than two firepoints
-			shipp->flags.remove(Ship_Flags::Secondary_dual_fire);
 			start_slot = swp->secondary_next_slot[bank];
 			end_slot = start_slot;
 		}
@@ -14749,7 +14825,7 @@ int ship_fire_secondary( object *obj, int allow_swarm, bool rollback_shot )
 		swp = &Player_ship->weapons;
 		if (bank >= 0) {
 			wip = &Weapon_info[swp->secondary_bank_weapons[bank]];
-			if (Player_ship->flags[Ship_Flags::Secondary_dual_fire]){
+			if (Player_ship->flags[Ship_Flags::Secondary_dual_fire] && ship_secondary_bank_can_dual_fire(Player_ship, bank)){
 				joy_ff_play_secondary_shoot((int) (wip->cargo_size * 2.0f));
 			} else {
 				joy_ff_play_secondary_shoot((int) wip->cargo_size);
@@ -15338,6 +15414,34 @@ void wing_bash_ship_name(ship *shipp, const wing *wingp, int ordinal, bool reset
 	}
 }
 
+bool wing_bash_legacy_hashed_ship_name(SCP_string &dest, const char *src)
+{
+	// missions might have ships within wings that were saved using the legacy hash format, with the hash suffix at the end
+	auto hash = get_pointer_to_first_hash_symbol(src);
+	if (hash && *(hash + 1) != '\0')
+	{
+		// find the run of digits immediately preceding the hash
+		auto digits = hash;
+		while (digits > src && isdigit(static_cast<unsigned char>(*(digits - 1))))
+			digits--;
+
+		// the ordinal must be at least one digit, preceded by a space, preceded by the wing name
+		if (digits < hash && digits > (src + 1) && *(digits - 1) == ' ')
+		{
+			// move the ordinal from before the hash to the end of the name
+			dest.assign(src, digits - 1);
+			dest += hash;
+			dest += ' ';
+			dest.append(digits, hash);
+
+			// we changed it
+			return true;
+		}
+	}
+
+	return false;
+}
+
 /**
  * Return the object index of the ship with name *name.
  */
@@ -15561,10 +15665,7 @@ int ship_info_lookup(const char *token)
 	return ship_info_lookup_sub(name);
 }
 
-/**
- * Return the ship index of the ship with name *name.
- */
-int ship_name_lookup(const char *name, int inc_players)
+static int ship_name_lookup_sub(const char *name, int inc_players)
 {
 	Assertion(name != nullptr, "NULL name passed to ship_name_lookup");
 
@@ -15582,7 +15683,26 @@ int ship_name_lookup(const char *name, int inc_players)
 	return -1;
 }
 
-int ship_type_name_lookup_sub(const char *name)
+/**
+ * Return the ship index of the ship with name *name.
+ */
+int ship_name_lookup(const char *name, int inc_players)
+{
+	// try the normal lookup
+	auto idx = ship_name_lookup_sub(name, inc_players);
+	if (idx >= 0)
+		return idx;
+
+	// also search for ship names hashed using the legacy format
+	SCP_string legacy_hashed;
+	if (wing_bash_legacy_hashed_ship_name(legacy_hashed, name))
+		return ship_name_lookup_sub(legacy_hashed.c_str(), inc_players);
+
+	// couldn't find it
+	return -1;
+}
+
+static int ship_type_name_lookup_sub(const char *name)
 {
 	Assertion(name != nullptr, "NULL name passed to ship_type_name_lookup");
 

code/ship/ship.h

@@ -973,6 +973,7 @@ extern const ship_registry_entry *ship_registry_get(const char *name);
 extern const ship_registry_entry *ship_registry_get(const SCP_string &name);
 extern const ship_registry_entry *ship_registry_get(int index);
 extern const ship_registry_entry *ship_registry_get(anchor_t anchor);
+extern void ship_registry_rename(int entry_index, const char *new_name, bool erase_old_key);
 
 #define REGULAR_WEAPON	(1<<0)
 #define DOGFIGHT_WEAPON (1<<1)
@@ -1681,13 +1682,14 @@ extern SCP_vector<wing_formation> Wing_formations;
 
 // Use the below macros when you want to find the index of an array element in the
 // Wings[] or Ships[] arrays.
-#define WING_INDEX(wingp) (static_cast<int>(wingp-Wings))
-#define SHIP_INDEX(shipp) (static_cast<int>(shipp-Ships))
-#define SHIP_REGISTRY_INDEX(ship_entry) (static_cast<int>(ship_entry-Ship_registry.data()))
+#define WING_INDEX(wingp) (static_cast<int>((wingp)-Wings))
+#define SHIP_INDEX(shipp) (static_cast<int>((shipp)-Ships))
+#define SHIP_REGISTRY_INDEX(ship_entry) (static_cast<int>((ship_entry)-Ship_registry.data()))
 
 
 extern void ship_init();				// called once	at game start
 extern void ship_level_init();		// called before the start of each level
+extern void ship_level_close();		// called after the end of each level
 
 //returns -1 if failed
 extern int ship_create(matrix* orient, vec3d* pos, int ship_type, const char* ship_name = nullptr, bool standalone_ship = false);
@@ -1700,13 +1702,14 @@ extern void ship_delete( object * objp );
 extern int ship_check_collision_fast( object * obj, object * other_obj, vec3d * hitpos );
 extern int ship_get_num_ships();
 
-#define SHIP_VANISHED			(1<<0)
-#define SHIP_DESTROYED			(1<<1)
-#define SHIP_DEPARTED_WARP		(1<<2)
-#define SHIP_DEPARTED_BAY		(1<<3)
-#define SHIP_DEPARTED			( SHIP_DEPARTED_BAY | SHIP_DEPARTED_WARP )
-#define SHIP_DESTROYED_REDALERT	(1<<4)
-#define SHIP_DEPARTED_REDALERT	(1<<5)
+#define SHIP_VANISHED               (1<<0)
+#define SHIP_DESTROYED              (1<<1)
+#define SHIP_DEPARTED_WARP          (1<<2)
+#define SHIP_DEPARTED_BAY           (1<<3)
+#define SHIP_DEPARTED               ( SHIP_DEPARTED_BAY | SHIP_DEPARTED_WARP )
+#define SHIP_DESTROYED_REDALERT     (1<<4)
+#define SHIP_DEPARTED_REDALERT      (1<<5)
+#define SHIP_PRESENT_AT_MISSION_END (1<<6)
 
 /**
  * @brief Deletes and de-inits a ship.
@@ -1730,6 +1733,7 @@ extern bool in_autoaim_fov(ship *shipp, int bank_to_fire, object *obj);
 extern int ship_stop_fire_primary(object * obj);
 extern int ship_fire_primary(object * objp, int force = 0, bool rollback_shot = false);
 extern int ship_fire_secondary(object * objp, int allow_swarm = 0, bool rollback_shot = false );
+extern bool ship_secondary_bank_can_dual_fire(const ship *shipp, int bank);
 bool ship_start_secondary_fire(object* objp);
 bool ship_stop_secondary_fire(object* objp);
 extern int ship_launch_countermeasure(object *objp, int rand_val = -1);
@@ -1780,6 +1784,8 @@ extern void wing_bash_ship_name(SCP_string &ship_name, const char *wing_name, in
 extern void wing_bash_ship_name(char *ship_name, const char *wing_name, int ordinal);
 extern void wing_bash_ship_name(p_object *p_objp, const wing *wingp, int ordinal, bool reset_display_name_if_normal = false);
 extern void wing_bash_ship_name(ship *shipp, const wing *wingp, int ordinal, bool reset_display_name_if_normal = false);
+extern bool wing_bash_legacy_hashed_ship_name(SCP_string &dest, const char *src);
+
 extern int Player_ship_class;
 
 //	Do the special effect for energy dissipating into the shield for a hit.

code/ship/shiphit.cpp

@@ -1222,6 +1222,8 @@ static void shiphit_record_player_killer(const object *killer_objp, player *p)
 		break;
 
 	case OBJ_PROP:
+		p->killer_objtype = OBJ_PROP;
+		p->killer_weapon_index = -1;
 		strcpy_s(p->killer_parent_name, "");
 		p->killer_species = -1;
 		p->killer_parent_name[0] = '\0';

code/utils/Random.cpp

@@ -1,4 +1,5 @@
 #include "Random.h"
+#include "RandomRange.h"
 
 #include <limits>
 #include <random>
@@ -8,6 +9,9 @@
 #include "globalincs/pstypes.h"
 
 namespace util {
+
+thread_local std::mt19937 seeder {std::random_device()()};
+
 namespace {
 template <typename RngType>
 class RandomImpl {

code/utils/RandomRange.h

@@ -54,6 +54,10 @@ size_t parse_number_list(T (&list)[N])
 }
 } // namespace
 
+//Sampling a random_device is REALLY expensive.
+//Instead of sampling one for each seed, create a pseudorandom seeder which is initialized ONCE from a random_device.
+extern thread_local std::mt19937 seeder;
+
 /**
  * @brief Generic class for generating numbers in a specific range
  *
@@ -77,10 +81,6 @@ class RandomRange {
 	ValueType m_minValue;
 	ValueType m_maxValue;
 
-	//Sampling a random_device is REALLY expensive.
-	//Instead of sampling one for each seed, create a pseudorandom seeder which is initialized ONCE from a random_device.
-	inline static thread_local std::mt19937 seeder {std::random_device()()};
-
   public:
 	template <typename T, typename... Ts, typename = typename std::enable_if<(sizeof... (Ts) >=1 || !std::is_convertible<T, ValueType>::value) && !std::is_same_v<std::decay_t<T>, RandomRange>, int>::type>
 	explicit RandomRange(T&& distributionFirstParameter, Ts&&... distributionParameters)

code/weapon/beam.h

@@ -226,7 +226,7 @@ typedef struct beam {
 extern std::array<beam, MAX_BEAMS> Beams;				// all beams
 extern int Beam_count;
 
-#define BEAM_INDEX(beam)			(int)((beam) - Beams.data())
+#define BEAM_INDEX(beam)			(static_cast<int>((beam)-Beams.data()))
 
 // ------------------------------------------------------------------------------------------------
 // BEAM WEAPON FUNCTIONS

code/weapon/shockwave.cpp

@@ -42,7 +42,7 @@ int Shockwave_inited = 0;
 // -----------------------------------------------------------
 // Function macros
 // -----------------------------------------------------------
-#define SW_INDEX(sw) (sw-Shockwaves)
+#define SW_INDEX(sw) (static_cast<int>((sw)-Shockwaves))
 	
 // -----------------------------------------------------------
 // Externals
@@ -308,7 +308,13 @@ void shockwave_move(object *shockwave_objp, float frametime)
 		CLAMP(sw->radius, 0.0f, sw->outer_radius);
 	}
 
-	if ( sw->time_elapsed > sw->total_time ) {
+	// It is possible the shockwave may have lifetime shorter than one frame (e.g. high speed, small radius), 
+	// which by default results in shockwave applying no damage.
+	// Provide optional fix to ensure shockwave is not killed until after this frame's damage pass has run.
+	bool sw_expired = sw->time_elapsed > sw->total_time;
+	bool sw_expire_fix = The_mission.ai_profile->flags[AI::Profile_Flags::Fix_shockwave_expire_before_do_damage];
+
+	if ( sw_expired && !sw_expire_fix ) {
         shockwave_objp->flags.set(Object::Object_Flags::Should_be_dead);
 		return;
 	}
@@ -443,6 +449,12 @@ void shockwave_move(object *shockwave_objp, float frametime)
 		}
 
 	}	// end for
+
+	// See the sw_expired check above: shockwave has now applied its final damage pass this frame, so let it die.  
+	// With the flag off we already returned before the loop.
+	if ( sw_expired && sw_expire_fix ) {
+        shockwave_objp->flags.set(Object::Object_Flags::Should_be_dead);
+	}
 }
 
 /**

code/weapon/swarm.cpp

@@ -387,7 +387,7 @@ void turret_swarm_set_up_info(int parent_objnum, ship_subsys *turret, const weap
 	int bank_fired = swp->current_secondary_bank;
 
 	// initialize tsi
-	tsi->weapon_class = weapon_info_get_index(wip);
+	tsi->weapon_class = WEAPON_INFO_INDEX(wip);
 	if (wip->wi_flags[Weapon::Info_Flags::Swarm]) {
 		tsi->num_to_launch = wip->swarm_count;
 	} else {

code/weapon/weapon.h

@@ -975,7 +975,8 @@ extern int Default_cmeasure_index;
 
 extern SCP_vector<int> Player_weapon_precedence;	// Vector of weapon types, precedence list for player weapon selection
 
-#define WEAPON_INDEX(wp)			(int)(wp-Weapons)
+#define WEAPON_INDEX(wp)			(static_cast<int>((wp)-Weapons))
+#define WEAPON_INFO_INDEX(wip)		(static_cast<int>((wip)-Weapon_info.data()))
 
 typedef struct tracking_info {
 	ship_subsys *subsys;
@@ -986,7 +987,6 @@ typedef struct tracking_info {
 } tracking_info;
 
 int weapon_info_lookup(const char *name);
-int weapon_info_get_index(const weapon_info *wip);
 
 inline int weapon_info_size()
 {

code/weapon/weapons.cpp

@@ -389,17 +389,6 @@ int weapon_info_lookup(const char *name)
 	return -1;
 }
 
-/**
- * Return the index of Weapon_info used by this pointer.  Equivalent to the old WEAPON_INFO_INDEX macro:
- * #define WEAPON_INFO_INDEX(wip)		(int)(wip-Weapon_info)
- */
-int weapon_info_get_index(const weapon_info *wip)
-{
-	Assertion(wip != nullptr, "NULL wip passed to weapon_info_get_index");
-	const weapon_info *data = Weapon_info.data();
-	return static_cast<int>(std::distance(data, wip));
-}
-
 //	Parse the weapon flags.
 void parse_wi_flags(weapon_info *weaponp)
 {
@@ -3893,7 +3882,7 @@ int parse_weapon(int subtype, bool replace, const char *filename)
 				wip->particle_spewers[spew_index] = particle::util::parseEffect(wip->name);
 			}
 			else { // we have a valid index, now parse the spewer already
-				auto& pspew_buffer = pspew_legacy_parse_data_buffer[weapon_info_get_index(wip)][spew_index];
+				auto& pspew_buffer = pspew_legacy_parse_data_buffer[WEAPON_INFO_INDEX(wip)][spew_index];
 				if (pspew_buffer.particle_spew_type == PSPEW_NONE) {
 					//This must be an uninitialized effect, store defaults.
 					pspew_buffer.particle_spew_count = 1;
@@ -3996,7 +3985,7 @@ int parse_weapon(int subtype, bool replace, const char *filename)
 			if (wip->particle_spewers.empty()) {
 				wip->particle_spewers.emplace_back(particle::ParticleEffectHandle::invalid());
 			}
-			auto& pspew_buffer = pspew_legacy_parse_data_buffer[weapon_info_get_index(wip)][0];
+			auto& pspew_buffer = pspew_legacy_parse_data_buffer[WEAPON_INFO_INDEX(wip)][0];
 			pspew_buffer.particle_spew_count = 1;
 			pspew_buffer.particle_spew_time = 25;
 			pspew_buffer.particle_spew_vel = 0.4f;
@@ -7147,9 +7136,15 @@ int weapon_create( const vec3d *pos, const matrix *porient, int weapon_type, int
 		Assertion( position != NULL, "'%s' is trying to fire a weapon that is not selected", Ships[parent_objp->instance].ship_name );
 
 		size_t curr_pos = *position;
-		if (((Weapon_info[weapon_type].subtype == WP_LASER && parent_shipp->flags[Ship::Ship_Flags::Primary_linked]) ||
-			 (Weapon_info[weapon_type].subtype == WP_MISSILE && parent_shipp->flags[Ship::Ship_Flags::Secondary_dual_fire])) &&
-			 (curr_pos > 0)) {
+		// only advance the pattern once per linked/paired shot rather than once per projectile
+		bool paired_shot = false;
+		if (Weapon_info[weapon_type].subtype == WP_LASER) {
+			paired_shot = parent_shipp->flags[Ship::Ship_Flags::Primary_linked];
+		} else if (Weapon_info[weapon_type].subtype == WP_MISSILE && !src_turret && parent_shipp->flags[Ship::Ship_Flags::Secondary_dual_fire]) {
+			// for dual fire, also check whether the armed bank can actually use it, since the flag is ignored rather than cleared for banks that can't
+			paired_shot = ship_secondary_bank_can_dual_fire(parent_shipp, parent_shipp->weapons.current_secondary_bank);
+		}
+		if (paired_shot && (curr_pos > 0)) {
 			curr_pos--;
 		}
 		++(*position);
@@ -7478,9 +7473,6 @@ int weapon_create( const vec3d *pos, const matrix *porient, int weapon_type, int
 
 	if (wip->wi_flags[Weapon::Info_Flags::Local_ssm])
 	{
-
-		Assert(parent_objp);		//local ssms must have a parent
-
 		wp->lssm_warpout_time=timestamp(wip->lssm_warpout_delay);
 		wp->lssm_warpin_time=timestamp(wip->lssm_warpout_delay + wip->lssm_warpin_delay);
 		wp->lssm_stage=1;
@@ -7585,7 +7577,7 @@ int weapon_create( const vec3d *pos, const matrix *porient, int weapon_type, int
 
 	Num_weapons++;
 
-	if (Weapons_inherit_parent_collision_group) {
+	if (Weapons_inherit_parent_collision_group && parent_objnum >= 0) {
 		Objects[objnum].collision_group_id = Objects[parent_objnum].collision_group_id;
 	}
 
@@ -7625,7 +7617,7 @@ int weapon_create( const vec3d *pos, const matrix *porient, int weapon_type, int
 	}
 
 	if (scripting::hooks::OnWeaponCreated->isActive()) {
-		scripting::hooks::OnWeaponCreated->run(scripting::hooks::WeaponCreatedConditions{ wp, &Objects[parent_objnum] },
+		scripting::hooks::OnWeaponCreated->run(scripting::hooks::WeaponCreatedConditions{ wp, parent_objnum < 0 ? nullptr : &Objects[parent_objnum] },
 			scripting::hook_param_list(
 				scripting::hook_param("Weapon", 'o', &Objects[objnum])
 			));

fred2/briefingeditordlg.cpp

@@ -432,7 +432,18 @@ void briefing_editor_dlg::update_data(int update)
 
 		if (m_last_icon >= 0) {
 			valid = (m_id != ptr->icons[m_last_icon].id);
-			if (m_id >= 0) {
+			if (m_id < 0) {
+				// briefing icon ids must never be negative; revert to the previous id
+				if (valid) {
+					char msg[1024];
+
+					sprintf(msg, "Icon ID #%d is invalid.  Briefing icon IDs cannot be negative.\n"
+						"Icon ID has been reset back to %d", m_id, ptr->icons[m_last_icon].id);
+
+					m_id = ptr->icons[m_last_icon].id;
+					MessageBox(msg);
+				}
+			} else {
 				if (valid && !m_change_local) {
 					for (i=m_last_stage+1; i<Briefing->num_stages; i++) {
 						if (find_icon(m_id, i) >= 0) {
@@ -473,7 +484,7 @@ void briefing_editor_dlg::update_data(int update)
 
 			string_copy(buf, m_icon_label, MAX_LABEL_LEN - 1);
 			lcl_fred_replace_stuff(buf, MAX_LABEL_LEN - 1);
-			if (stricmp(ptr->icons[m_last_icon].label, buf) && !m_change_local) {
+			if (strcmp(ptr->icons[m_last_icon].label, buf) && !m_change_local) {
 				set_modified();
 				reset_icon_loop(m_last_stage);
 				while (get_next_icon(m_id))
@@ -483,7 +494,7 @@ void briefing_editor_dlg::update_data(int update)
 
 			string_copy(buf, m_icon_closeup_label, MAX_LABEL_LEN - 1);
 			lcl_fred_replace_stuff(buf, MAX_LABEL_LEN - 1);
-			if (stricmp(ptr->icons[m_last_icon].closeup_label, buf) && !m_change_local) {
+			if (strcmp(ptr->icons[m_last_icon].closeup_label, buf) && !m_change_local) {
 				set_modified();
 				reset_icon_loop(m_last_stage);
 				while (get_next_icon(m_id))
@@ -491,28 +502,33 @@ void briefing_editor_dlg::update_data(int update)
 			}
 			strcpy_s(ptr->icons[m_last_icon].closeup_label, buf);
 
-			if (m_icon_scale > 0)
-				ptr->icons[m_last_icon].scale_factor = m_icon_scale / 100.0f;
+			if (m_icon_scale > 0) {
+				float new_scale = m_icon_scale / 100.0f;
+				if ((ptr->icons[m_last_icon].scale_factor != new_scale) && !m_change_local) {
+					set_modified();
+					reset_icon_loop(m_last_stage);
+					while (get_next_icon(m_id))
+						iconp->scale_factor = new_scale;
+				}
+				ptr->icons[m_last_icon].scale_factor = new_scale;
+			}
 
-			if ( m_hilight )
-				ptr->icons[m_last_icon].flags |= BI_HIGHLIGHT;
-			else
-				ptr->icons[m_last_icon].flags &= ~BI_HIGHLIGHT;
+			// build the new flags for this icon from the checkboxes, preserving any non-editable bits
+			const int editable_flags = BI_HIGHLIGHT | BI_MIRROR_ICON | BI_USE_WING_ICON | BI_USE_CARGO_ICON;
+			int new_flags = ptr->icons[m_last_icon].flags;
+			if (m_hilight)   new_flags |= BI_HIGHLIGHT;      else new_flags &= ~BI_HIGHLIGHT;
+			if (m_flipicon)  new_flags |= BI_MIRROR_ICON;    else new_flags &= ~BI_MIRROR_ICON;
+			if (m_use_wing)  new_flags |= BI_USE_WING_ICON;  else new_flags &= ~BI_USE_WING_ICON;
+			if (m_use_cargo) new_flags |= BI_USE_CARGO_ICON; else new_flags &= ~BI_USE_CARGO_ICON;
 
-			if (m_flipicon)
-				ptr->icons[m_last_icon].flags |= BI_MIRROR_ICON;
-			else
-				ptr->icons[m_last_icon].flags &= ~BI_MIRROR_ICON;
-
-			if (m_use_wing)
-				ptr->icons[m_last_icon].flags |= BI_USE_WING_ICON;
-			else
-				ptr->icons[m_last_icon].flags &= ~BI_USE_WING_ICON;
-
-			if (m_use_cargo)
-				ptr->icons[m_last_icon].flags |= BI_USE_CARGO_ICON;
-			else
-				ptr->icons[m_last_icon].flags &= ~BI_USE_CARGO_ICON;
+			if ((ptr->icons[m_last_icon].flags != new_flags) && !m_change_local) {
+				set_modified();
+				reset_icon_loop(m_last_stage);
+				// apply only the editable bits to later icons, preserving their other flags
+				while (get_next_icon(m_id))
+					iconp->flags = (iconp->flags & ~editable_flags) | (new_flags & editable_flags);
+			}
+			ptr->icons[m_last_icon].flags = new_flags;
 
 			if ((ptr->icons[m_last_icon].type != m_icon_image) && !m_change_local) {
 				set_modified();
@@ -942,6 +958,7 @@ void briefing_editor_dlg::copy_stage(int from, int to)
 		Briefing->stages[to].camera_time = 500;
 		Briefing->stages[to].num_icons = 0;
 		Briefing->stages[to].formula = Locked_sexp_true;
+		Briefing->stages[to].draw_grid = true;
 		Briefing->stages[to].grid_color = Color_briefing_grid;
 		return;
 	}
@@ -956,6 +973,7 @@ void briefing_editor_dlg::copy_stage(int from, int to)
 	Briefing->stages[to].num_icons = Briefing->stages[from].num_icons;
 	Briefing->stages[to].num_lines = Briefing->stages[from].num_lines;
 	Briefing->stages[to].formula = Briefing->stages[from].formula;
+	Briefing->stages[to].draw_grid = Briefing->stages[from].draw_grid;
 	// For now let's just always set this back to default. Eventually when we have a UI color picker in qtFRED, we can copy from stage to stage
 	Briefing->stages[to].grid_color = Color_briefing_grid;
 
@@ -1089,12 +1107,13 @@ void briefing_editor_dlg::OnMakeIcon()
 
 	strncpy(biconp->label, name, len);
 	biconp->label[len] = 0;
+	biconp->closeup_label[0] = 0;
 //	iconp->text[0] = 0;
 	biconp->type = 0;
 	biconp->team = team;
 	biconp->pos = pos;
 	biconp->flags = 0;
-	biconp->id = Cur_brief_id++;
+	biconp->id = get_unused_briefing_icon_id();
 	biconp->scale_factor = 1.0f;
 
 	biconp->modelnum = -1;
@@ -1198,7 +1217,7 @@ void briefing_editor_dlg::OnDeleteIcon()
 
 void briefing_editor_dlg::delete_icon(int num)
 {
-	int i, z;
+	int i, l, z;
 
 	if (num < 0)
 		num = m_cur_icon;
@@ -1217,6 +1236,24 @@ void briefing_editor_dlg::delete_icon(int num)
 	m_cur_icon = -1;
 	update_data(1);
 	obj_delete(icon_obj[num]);
+
+	// remove any lines that reference the icon being deleted
+	i = Briefing->stages[m_cur_stage].num_lines;
+	while (i--)
+		if ((Briefing->stages[m_cur_stage].lines[i].start_icon == num) || (Briefing->stages[m_cur_stage].lines[i].end_icon == num)) {
+			Briefing->stages[m_cur_stage].num_lines--;
+			for (l=i; l<Briefing->stages[m_cur_stage].num_lines; l++)
+				Briefing->stages[m_cur_stage].lines[l] = Briefing->stages[m_cur_stage].lines[l+1];
+		}
+
+	// fix the indexes of lines that reference the icons being shifted down
+	for (i=0; i<Briefing->stages[m_cur_stage].num_lines; i++) {
+		if (Briefing->stages[m_cur_stage].lines[i].start_icon > num)
+			Briefing->stages[m_cur_stage].lines[i].start_icon--;
+		if (Briefing->stages[m_cur_stage].lines[i].end_icon > num)
+			Briefing->stages[m_cur_stage].lines[i].end_icon--;
+	}
+
 	for (i=num+1; i<Briefing->stages[m_cur_stage].num_icons; i++) {
 		Briefing->stages[m_cur_stage].icons[i-1] = Briefing->stages[m_cur_stage].icons[i];
 		icon_obj[i-1] = icon_obj[i];
@@ -1282,10 +1319,13 @@ int briefing_editor_dlg::check_mouse_hit(int x, int y)
 	return -1;
 }
 
-void briefing_editor_dlg::OnPropagateIcons() 
+void briefing_editor_dlg::OnPropagateIcons()
 {
 	object *ptr;
 
+	// commit any pending edits in the icon fields before propagating the icons forward
+	update_data(1);
+
 	ptr = GET_FIRST(&obj_used_list);
 	while (ptr != END_OF_LIST(&obj_used_list)) {
 		if ((ptr->type == OBJ_POINT) && (ptr->flags[Object::Object_Flags::Marked])) {
@@ -1325,6 +1365,26 @@ int briefing_editor_dlg::find_icon(int id, int stage)
 	return -1;
 }
 
+// is the given id already used by any icon in any stage of the current briefing?
+bool briefing_editor_dlg::briefing_icon_id_used(int id)
+{
+	for (int s=0; s<Briefing->num_stages; s++)
+		if (find_icon(id, s) >= 0)
+			return true;
+
+	return false;
+}
+
+// find an id that isn't already used by an icon in the current briefing, advancing
+// Cur_brief_id past any values that are already taken (e.g. from manual id edits)
+int briefing_editor_dlg::get_unused_briefing_icon_id()
+{
+	while (briefing_icon_id_used(Cur_brief_id))
+		Cur_brief_id++;
+
+	return Cur_brief_id++;
+}
+
 void briefing_editor_dlg::reset_icon_loop(int stage)
 {
 	stage_loop = stage + 1;
@@ -1337,7 +1397,7 @@ int briefing_editor_dlg::get_next_icon(int id)
 		icon_loop++;
 		if (icon_loop >= Briefing->stages[stage_loop].num_icons) {
 			stage_loop++;
-			if (stage_loop > Briefing->num_stages)
+			if (stage_loop >= Briefing->num_stages)
 				return 0;
 
 			icon_loop = -1;

fred2/briefingeditordlg.h

@@ -34,6 +34,8 @@ public:
 	void OnOK();
 	void OnCancel();
 	int find_icon(int id, int stage);
+	bool briefing_icon_id_used(int id);
+	int get_unused_briefing_icon_id();
 	void propagate_icon(int num);
 	void reset_editor();
 	int check_mouse_hit(int x, int y);

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 @@ protected:
 	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/freddoc.cpp

@@ -36,6 +36,7 @@
 #include "localization/fhash.h"
 #include "localization/localize.h"
 #include "mission/missiongoals.h"
+#include "mission/missiongrid.h"
 #include "mission/missionparse.h"
 #include "object/object.h"
 #include "render/3d.h"
@@ -233,9 +234,20 @@ bool CFREDDoc::load_mission(const char *pathname, int flags) {
 	Parse_viewer_pos = view_pos;
 	Parse_viewer_orient = view_orient;
 
+	// preserve the editor grid across the reload; it's display state, not part of the mission,
+	// but clear_mission() recreates it via fred_render_init().  (See the view_pos/view_orient handling above and below.)
+	matrix grid_orient = The_grid->gmatrix;
+	vec3d grid_center = The_grid->center;
+	int grid_nrows = The_grid->nrows;
+	int grid_ncols = The_grid->ncols;
+	float grid_square_size = The_grid->square_size;
+
 	// activate the localizer hash table
 	fhash_flush();
 
+	// Guard against reentrant ship-editor write-back while the mission is loaded.  The guard is released once the mission has stabilized.
+	Ship_editor_dialog.bypass_all++;
+
 	clear_mission(flags & MPF_FAST_RELOAD);
 
 	// message 1: required version
@@ -250,6 +262,7 @@ bool CFREDDoc::load_mission(const char *pathname, int flags) {
 		}
 
 		Fred_view_wnd->MessageBox(name);
+		Ship_editor_dialog.bypass_all--;
 		create_new_mission();
 		return false;
 	}
@@ -392,9 +405,16 @@ bool CFREDDoc::load_mission(const char *pathname, int flags) {
 
 	view_pos = Parse_viewer_pos;
 	view_orient = Parse_viewer_orient;
+
+	// restore the editor grid that was preserved above
+	create_grid(The_grid, &grid_orient.vec.fvec, &grid_orient.vec.rvec, &grid_center, grid_nrows, grid_ncols, grid_square_size);
+
 	set_modified(0);
 	stars_post_level_init();
 
+	// mission is fully built and consistent now; allow the editors to repopulate
+	Ship_editor_dialog.bypass_all--;
+
 	recreate_dialogs();
 
 	// This hook will allow for scripts to know when a mission has been loaded

fred2/fredrender.cpp

@@ -1424,21 +1424,29 @@ int object_check_collision(object *objp, vec3d *p0, vec3d *p1, vec3d *hitpos) {
 			return 0;
 	}
 
-	// PROP HERE?
+	if (objp->type == OBJ_PROP) {
+		if (!Props[objp->instance].has_value())
+			return 0;
+	}
 
 	if (objp->flags[Object::Object_Flags::Hidden, Object::Object_Flags::Locked_from_editing])
 		return 0;
 
-	if ((Show_ship_models || Show_outlines) && (objp->type == OBJ_SHIP)) {
-		mc.model_num = Ship_info[Ships[objp->instance].ship_info_index].model_num;			// Fill in the model to check
+	mc.model_instance_num = -1;
 
-	} else if ((Show_ship_models || Show_outlines) && (objp->type == OBJ_START)) {
-		mc.model_num = Ship_info[Ships[objp->instance].ship_info_index].model_num;			// Fill in the model to check
+	if ((Show_ship_models || Show_outlines) && (objp->type == OBJ_SHIP || objp->type == OBJ_START)) {
+		auto& shp = Ships[objp->instance];
+		mc.model_num = Ship_info[shp.ship_info_index].model_num;			// Fill in the model to check
+		mc.model_instance_num = shp.model_instance_num;
+
+	} else if ((Show_ship_models || Show_outlines) && (objp->type == OBJ_PROP)) {
+		auto& prp = Props[objp->instance].value();
+		mc.model_num = Prop_info[prp.prop_info_index].model_num;			// Fill in the model to check
+		mc.model_instance_num = prp.model_instance_num;
 
 	} else
 		return fvi_ray_sphere(hitpos, p0, p1, &objp->pos, (objp->radius > 0.1f) ? objp->radius : LOLLIPOP_SIZE);
 
-	mc.model_instance_num = -1;
 	mc.orient = &objp->orient;	// The object's orient
 	mc.pos = &objp->pos;			// The object's position
 	mc.p0 = p0;					// Point 1 of ray to check

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"
@@ -2466,7 +2467,7 @@ int query_single_wing_marked()
 void CFREDView::OnDisbandWing() 
 {
 	if (query_single_wing_marked()) {
-		remove_wing(cur_wing);
+		disband_wing(cur_wing);
 		FREDDoc_ptr->autosave("wing disband");
 
 	} else
@@ -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;
@@ -3590,6 +3596,7 @@ int CFREDView::fred_check_sexp(int sexp, int type, const char *location, ...)
 			return 1;
 	}
 
+	z = 0;
 	if (Error_checker_checks_potential_issues || Error_checker_checks_potential_issues_once)
 		z = check_sexp_potential_issues(sexp, &faulty_node, issue_msg);
 	if (z)

fred2/management.cpp

@@ -775,7 +775,7 @@ int create_object(vec3d *pos, int waypoint_instance, bool prop)
 	int obj, n;
 
 	if (prop) {
-		int prop_class = m_new_prop_type_combo_box.GetCurSel();
+		int prop_class = m_new_prop_type_combo_box.GetPropClass(m_new_prop_type_combo_box.GetCurSel());
 		if (prop_class < 0 || prop_class >= prop_info_size())
 			return -1;
 
@@ -858,11 +858,16 @@ void create_new_mission()
 
 void reset_mission()
 {
+	// Guard against reentrant ship-editor write-back while the mission is reset.
+	Ship_editor_dialog.bypass_all++;
+
 	clear_mission();
 
 	create_player(&vmd_zero_vector, &vmd_identity_matrix);
 
 	stars_post_level_init();
+
+	Ship_editor_dialog.bypass_all--;
 }
 
 void clear_mission(bool fast_reload)
@@ -1235,7 +1240,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 +1275,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/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 @@ protected:
 	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);

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,12 +2440,22 @@ 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);
 			}
@@ -2461,6 +2463,9 @@ void CShipEditorDlg::OnSetAsPlayerShip()
 		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);
@@ -442,7 +443,7 @@ void remove_ship_from_wing(int ship, int min) {
 	rename_ship(ship, buf);
 }
 
-void remove_wing(int wing_num)
+void disband_wing(int wing_num)
 {
 	int i, total;
 	object *ptr;
@@ -450,6 +451,8 @@ void remove_wing(int wing_num)
 	if (check_wing_dependencies(wing_num))
 		return;
 
+	delete_reinforcement(Wings[wing_num].name);
+
 	Ship_editor_dialog.bypass_errors = Wing_editor_dialog.bypass_errors = 1;
 	Ship_editor_dialog.update_data(0);
 	total = Wings[wing_num].wave_count;

fred2/wing.h

@@ -27,7 +27,7 @@ int create_wing();
  *
  * @param[in] wing_num Index of the wing
  */
-void remove_wing(int wing_num);
+void disband_wing(int wing_num);
 
 /**
  * @brief Takes a ship out of a wing, deleting the wing if that was the only ship in it.

fred2/wing_editor.cpp

@@ -1078,7 +1078,7 @@ void wing_editor::OnDeleteWing()
 void wing_editor::OnDisbandWing()
 {
 	modified = 0;  // no need to run update checks, since wing will be gone shortly anyway.
-	remove_wing(cur_wing);
+	disband_wing(cur_wing);
 }
 
 void wing_editor::OnGoals2()
@@ -1475,6 +1475,9 @@ void wing_editor::OnWingFormationAlign()
 
 		get_absolute_wing_pos(&objp->pos, leader_objp, cur_wing, i, false);
 		objp->orient = leader_objp->orient;
+
+		// drag any docked partners along (no-op for undocked ships)
+		object_moved(objp);
 	}
 
 	// roll back temporary formation

freespace2/freespace.cpp

@@ -984,7 +984,7 @@ void game_level_close()
 		particle::ParticleManager::get()->clearSources();
 		particle::close();
 		trail_level_close();
-		ship_close_cockpit_displays(Player_ship);
+		ship_level_close();
 		hud_level_close();
 		hud_escort_clear_all();
 		model_instance_free_all();
@@ -2306,15 +2306,15 @@ void game_show_framerate()
 		MEMORYSTATUSEX mem_stats;
 		mem_stats.dwLength = sizeof(mem_stats);
 		if (GlobalMemoryStatusEx(&mem_stats)) {
-			sprintf(mem_buffer, "Physical Free: %" PRIu64 " / %" PRIu64 " Meg", mem_stats.ullAvailPhys / 1024 / 1024, mem_stats.ullTotalPhys / 1024 / 1024);
+			sprintf(mem_buffer, "Physical Free: " UINT64_T_ARG " / " UINT64_T_ARG " Meg", mem_stats.ullAvailPhys / 1024 / 1024, mem_stats.ullTotalPhys / 1024 / 1024);
 			gr_string(sx, sy, mem_buffer.c_str(), GR_RESIZE_NONE);
 			sy += line_height;
 
-			sprintf(mem_buffer, "Pagefile Free: %" PRIu64 " / %" PRIu64 " Meg", mem_stats.ullAvailPageFile / 1024 / 1024, mem_stats.ullTotalPageFile / 1024 / 1024);
+			sprintf(mem_buffer, "Pagefile Free: " UINT64_T_ARG " / " UINT64_T_ARG " Meg", mem_stats.ullAvailPageFile / 1024 / 1024, mem_stats.ullTotalPageFile / 1024 / 1024);
 			gr_string(sx, sy, mem_buffer.c_str(), GR_RESIZE_NONE);
 			sy += line_height;
 
-			sprintf(mem_buffer, "Virtual Free:  %" PRIu64 " / %" PRIu64 " Meg", mem_stats.ullAvailVirtual / 1024 / 1024, mem_stats.ullTotalVirtual / 1024 / 1024);
+			sprintf(mem_buffer, "Virtual Free:  " UINT64_T_ARG " / " UINT64_T_ARG " Meg", mem_stats.ullAvailVirtual / 1024 / 1024, mem_stats.ullTotalVirtual / 1024 / 1024);
 			gr_string(sx, sy, mem_buffer.c_str(), GR_RESIZE_NONE);
 		}
 	}

qtfred/src/mission/Editor.cpp

@@ -973,7 +973,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;
@@ -1012,17 +1011,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--;
 
@@ -1562,7 +1551,7 @@ int Editor::set_reinforcement(const char* name, int state) {
 
 	return 0;
 }
-void Editor::remove_wing(int wing_num) {
+void Editor::disband_wing(int wing_num) {
 
 	int i, total;
 	object* ptr;
@@ -1571,6 +1560,8 @@ void Editor::remove_wing(int wing_num) {
 		return;
 	}
 
+	delete_reinforcement(Wings[wing_num].name);
+
 	total = Wings[wing_num].wave_count;
 	for (i = 0; i < total; i++) {
 		ptr = &Objects[wing_objects[wing_num][i]];

qtfred/src/mission/Editor.h

@@ -233,7 +233,7 @@ class Editor : public QObject {
 	 *
 	 * @param[in] wing_num Index of the wing
 	 */
-	void remove_wing(int wing_num);
+	void disband_wing(int wing_num);
 
 	void delete_marked();
 

qtfred/src/mission/EditorViewport.cpp

@@ -712,15 +712,19 @@ int EditorViewport::object_check_collision(object* objp, vec3d* p0, vec3d* p1, v
 		return 0;
 	}
 
-	if ((view.Show_ship_models || view.Show_outlines) && (objp->type == OBJ_SHIP)) {
-		mc.model_num = Ship_info[Ships[objp->instance].ship_info_index].model_num; // Fill in the model to check
-	} else if ((view.Show_ship_models || view.Show_outlines) && (objp->type == OBJ_START)) {
-		mc.model_num = Ship_info[Ships[objp->instance].ship_info_index].model_num; // Fill in the model to check
+	mc.model_instance_num = -1;
+
+	if ((view.Show_ship_models || view.Show_outlines) && (objp->type == OBJ_SHIP || objp->type == OBJ_START)) {
+		auto& shp = Ships[objp->instance];
+		mc.model_num = Ship_info[shp.ship_info_index].model_num;			// Fill in the model to check
+		mc.model_instance_num = shp.model_instance_num;
+	} else if ((view.Show_ship_models || view.Show_outlines) && (objp->type == OBJ_PROP)) {
+		auto& prp = Props[objp->instance].value();
+		mc.model_num = Prop_info[prp.prop_info_index].model_num;			// Fill in the model to check
+		mc.model_instance_num = prp.model_instance_num;
 	} else {
 		return fvi_ray_sphere(hitpos, p0, p1, &objp->pos, (objp->radius > 0.1f) ? objp->radius : LOLLIPOP_SIZE);
 	}
-
-	mc.model_instance_num = -1;
 	mc.orient = &objp->orient; // The object's orient
 	mc.pos = &objp->pos; // The object's position
 	mc.p0 = p0; // Point 1 of ray to check

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/BriefingEditorDialogModel.cpp

@@ -695,9 +695,68 @@ int BriefingEditorDialogModel::getIconId() const
 	return s.icons[_currentIcon].id;
 }
 
-void BriefingEditorDialogModel::setIconId(int id)
+bool BriefingEditorDialogModel::setIconId(int id)
 {
-	applyToSelectedIconsCurrentAndForward([&](brief_icon& ic) { modify(ic.id, id); });
+	auto& briefing = _wipBriefings[_currentTeam];
+	if (briefing.num_stages <= 0 || _currentStage < 0 || _currentStage >= briefing.num_stages)
+		return true;
+
+	auto& stage = briefing.stages[_currentStage];
+	if (_currentIcon < 0 || _currentIcon >= stage.num_icons)
+		return true;
+
+	// an id applies to a single icon, so operate on the current icon rather than the
+	// whole selection; applying one id to several icons would itself create a collision
+	const int oldId = stage.icons[_currentIcon].id;
+	if (id == oldId)
+		return true; // no change
+
+	// briefing icon ids must never be negative (the spin box also enforces this)
+	if (id < 0)
+		return false;
+
+	// an icon id must be unique within its stage
+	for (int i = 0; i < stage.num_icons; ++i) {
+		if (i != _currentIcon && stage.icons[i].id == id) {
+			QMessageBox::warning(nullptr,
+				tr("Icon ID"),
+				tr("Icon ID %1 is already used by another icon in this stage.  The ID has not been changed.").arg(id));
+			return false;
+		}
+	}
+
+	// when propagating forward, the new id must not already be used by a different icon in a
+	// later stage, or the global rename would merge two distinct icon chains into one
+	if (!_changeLocally) {
+		for (int st = _currentStage + 1; st < briefing.num_stages; ++st) {
+			const auto& s = briefing.stages[st];
+			for (int i = 0; i < s.num_icons; ++i) {
+				if (s.icons[i].id == id) {
+					QMessageBox::warning(nullptr,
+						tr("Icon ID"),
+						tr("Icon ID %1 is already used in a later stage.  You can only change to that ID "
+						   "locally.  The ID has not been changed.").arg(id));
+					return false;
+				}
+			}
+		}
+	}
+
+	// apply the change to the current icon, and (unless local-only) to the same icon in later stages
+	stage.icons[_currentIcon].id = id;
+	if (!_changeLocally) {
+		for (int st = _currentStage + 1; st < briefing.num_stages; ++st) {
+			auto& s = briefing.stages[st];
+			for (int i = 0; i < s.num_icons; ++i) {
+				if (s.icons[i].id == oldId)
+					s.icons[i].id = id;
+			}
+		}
+	}
+
+	set_modified();
+	modelChanged();
+	return true;
 }
 
 SCP_string BriefingEditorDialogModel::getIconLabel() const

qtfred/src/mission/dialogs/BriefingEditorDialogModel.h

@@ -70,7 +70,8 @@ class BriefingEditorDialogModel : public AbstractDialogModel {
 	vec3d getIconPosition() const;
 	void setIconPosition(const vec3d& pos);
 	int getIconId() const;
-	void setIconId(int id);
+	// returns false if the requested id was rejected (e.g. it collides with another icon)
+	bool setIconId(int id);
 	SCP_string getIconLabel() const;
 	void setIconLabel(const SCP_string& text);
 	SCP_string getIconCloseupLabel() const;

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();