address issues with multi interpolation code

code/network/multi_interpolate.cpp

@@ -14,6 +14,8 @@ void interpolation_manager::reassess_packet_index(vec3d* pos, matrix* ori, physi
 	int current_index = static_cast<int>(_packets.size()) - 2;
 	int prev_index = static_cast<int>(_packets.size()) - 1;
 
+	const bool index_only = ( !pos || !ori || !pip );
+
 	// iterate through the packets
 	for (; current_index > -1; current_index--, prev_index--) {
 
@@ -25,7 +27,7 @@ void interpolation_manager::reassess_packet_index(vec3d* pos, matrix* ori, physi
 			// probably the "hackiest" thing about this.  If we were just straight simulating, 
 			// and we now need to go back, pretend that the position we were in *was* our old packet
 			// and we are now going towards our new packet's physics.
-			if (_simulation_mode) {
+			if (_simulation_mode && !index_only) {
 				replace_packet(prev_index, pos, ori, pip); // TODO, if simulation mode was forced by the collision code, this method regresses a bug where collisions instantly kill
 				_simulation_mode = false;
 			}
@@ -34,9 +36,11 @@ void interpolation_manager::reassess_packet_index(vec3d* pos, matrix* ori, physi
 		}
 	}
 
-	// if we didn't find indexes then we are overwhelmingly likely to have passed the server somehow
-	// and we need to make sure that we just straight simulate these ships
-	_simulation_mode = true;
+	if ( !index_only ) {
+		// if we didn't find indexes then we are overwhelmingly likely to have passed the server somehow
+		// and we need to make sure that we just straight simulate these ships
+		_simulation_mode = true;
+	}
 }
 
 void interpolate_main_helper(int objnum, vec3d* pos, matrix* ori, physics_info* pip, vec3d* last_pos, matrix* last_orient, vec3d* gravity, bool player_ship)
@@ -112,7 +116,7 @@ void interpolation_manager::interpolate_main(vec3d* pos, matrix* ori, physics_in
 	}
 
 	// calc what the current timing should be.
-	float numerator = static_cast<float>(_packets[_upcoming_packet_index].remote_missiontime) - static_cast<float>(Multi_Timing_Info.get_current_time());
+	float numerator = static_cast<float>(Multi_Timing_Info.get_current_time()) - static_cast<float>(_packets[_prev_packet_index].remote_missiontime);
 	float denominator = static_cast<float>(_packets[_upcoming_packet_index].remote_missiontime) - static_cast<float>(_packets[_prev_packet_index].remote_missiontime);
 
 	// work around for weird situations that might cause NAN (you just never know with multi)
@@ -162,7 +166,7 @@ void interpolation_manager::interpolate_main(vec3d* pos, matrix* ori, physics_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>(_packets[next_packet_index].remote_missiontime) - static_cast<float>(stamp.value());
+	float numerator = static_cast<float>(stamp.value()) - 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;
@@ -174,7 +178,7 @@ void interpolation_manager::reinterpolate_previous(TIMESTAMP stamp, int prev_pac
 
 	physics_snapshot temp_snapshot;
 
-	physics_interpolate_snapshots(temp_snapshot, _packets[_prev_packet_index].snapshot, _packets[_upcoming_packet_index].snapshot, scale);
+	physics_interpolate_snapshots(temp_snapshot, _packets[prev_packet_index].snapshot, _packets[next_packet_index].snapshot, scale);
 	physics_apply_snapshot_manual(position, orientation, velocity, rotational_velocity, temp_snapshot);
 }
 
@@ -213,21 +217,26 @@ void interpolation_manager::add_packet(int objnum, int frame, int packet_timesta
 				_prev_packet_index = 1;
 			}
 
-			if (static_cast<int>(_packets.size()) > PACKET_INFO_LIMIT) {
+			if (_packets.size() > PACKET_INFO_LIMIT) {
 				_packets.pop_back();
 			}
 
-			// whenenver the server gets a player packet, we need to update the ship record, since the old info is now stale
-			if (Objects[objnum].flags[Object::Object_Flags::Player_ship]){
+			if ((_upcoming_packet_index >= 0) && (_prev_packet_index >= 0)) {
+				// update packet indexes (ignoring simulation mode)
+				// NOTE: indexes must be valid before being reassesed!
+				reassess_packet_index();
 
-				int start_time = Multi_Timing_Info.get_mission_start_time();
+				// whenenver the server gets a player packet, we need to update the ship record, since the old info is now stale
+				if (Objects[objnum].flags[Object::Object_Flags::Player_ship]){
+					int start_time = Multi_Timing_Info.get_mission_start_time();
 
-				multi_ship_record_signal_update(objnum, TIMESTAMP(start_time + _packets[_prev_packet_index].remote_missiontime), TIMESTAMP(start_time + _packets[_upcoming_packet_index].remote_missiontime), _prev_packet_index, _upcoming_packet_index);
+					multi_ship_record_signal_update(objnum, TIMESTAMP(start_time + _packets[_prev_packet_index].remote_missiontime), TIMESTAMP(start_time + _packets[_upcoming_packet_index].remote_missiontime), _prev_packet_index, _upcoming_packet_index);
 
-				// if it's not the front packet, we need to update more info past the current packet, as well.  
-				// Should be rare though as it is a contingency for out of order packets.
-				if (_upcoming_packet_index != 0){
-					multi_ship_record_signal_update(objnum, TIMESTAMP(start_time + _packets[_upcoming_packet_index].remote_missiontime), TIMESTAMP(start_time + _packets[_upcoming_packet_index - 1].remote_missiontime), _upcoming_packet_index, _upcoming_packet_index - 1);
+					// if it's not the front packet, we need to update more info past the current packet, as well.
+					// Should be rare though as it is a contingency for out of order packets.
+					if (_upcoming_packet_index > 0) {
+						multi_ship_record_signal_update(objnum, TIMESTAMP(start_time + _packets[_upcoming_packet_index].remote_missiontime), TIMESTAMP(start_time + _packets[_upcoming_packet_index - 1].remote_missiontime), _upcoming_packet_index, _upcoming_packet_index - 1);
+					}
 				}
 			}
 
@@ -240,6 +249,11 @@ void interpolation_manager::add_packet(int objnum, int frame, int packet_timesta
 // should never replace index 0
 void interpolation_manager::replace_packet(int index, vec3d* pos, matrix* orient, physics_info* pip) 
 {
+	if (index < 1) {
+		UNREACHABLE("Invalid replace interpolation packet index! (%d)", index);
+		return;
+	}
+
 	// the hackiest part of the hack? Setting its frame. Let FSO think that it was basically brand new.
 	// it needs to handle it this way because otherwise another packet might get placed in front of it, 
 	// and we lose our intended effect of interpolating the simulation error away.

code/network/multi_interpolate.h

@@ -6,7 +6,7 @@
 
 struct physics_info;
 
-constexpr int PACKET_INFO_LIMIT = 4; // we should never need more than 4 packets to do interpolation.  Overwrite the oldest ones if we do.
+constexpr size_t PACKET_INFO_LIMIT = 4; // we should never need more than 4 packets to do interpolation.  Overwrite the oldest ones if we do.
 
 typedef struct packet_info {
 
@@ -42,7 +42,7 @@ class interpolation_manager {
 	SCP_vector<packet_info> _packets;	// all the info from the position/orientation portion of packets that we care to keep
 	int _source_player_index;
 
-	void reassess_packet_index(vec3d* pos, matrix* ori, physics_info* pip);		// for finding which packets from within _packets we should use
+	void reassess_packet_index(vec3d* pos = nullptr, matrix* ori = nullptr, physics_info* pip = nullptr);		// for finding which packets from within _packets we should use
 	void replace_packet(int index, vec3d* pos, matrix* orient, physics_info* pip);	// a function that acts as a workaround, when coming out of simulation_mode
 
 	// Frame numbers that helps us figure out if we should ignore new information coming from the server because
@@ -52,6 +52,7 @@ class interpolation_manager {
 	int _client_info_comparison_frame; 				// what frame was the last cleint info received?
 	SCP_vector<std::pair<int,int>> _subsystems_comparison_frame;	// what frame was the last subsystem information received? (for each subsystem) First is health, second is animation
 	int _ai_comparison_frame;						// what frame was the last ai information received?
+	int _support_comparison_frame;					// what frame was the last support information received?
 
 public:
 
@@ -85,6 +86,7 @@ class interpolation_manager {
 
 
 	int get_ai_comparison_frame() const { return _ai_comparison_frame; }
+	int get_support_comparison_frame() const { return _support_comparison_frame; }
 
 	void set_hull_comparison_frame(int frame) { _hull_comparison_frame = frame; }
 	void set_shields_comparison_frame(int frame) { _shields_comparison_frame = frame; }
@@ -105,6 +107,7 @@ class interpolation_manager {
 	}
 
 	void set_ai_comparison_frame(int frame) { _ai_comparison_frame = frame; }
+	void set_support_comparison_frame(int frame) { _support_comparison_frame = frame; }
 
 	void force_interpolation_mode() { _simulation_mode = true; }
 
@@ -134,6 +137,7 @@ class interpolation_manager {
 		}
 
 		_ai_comparison_frame = -1;
+		_support_comparison_frame = -1;
 	}
 
 	void clean_up() 
@@ -155,6 +159,7 @@ class interpolation_manager {
 		_shields_comparison_frame = -1;
 		_client_info_comparison_frame = -1;
 		_ai_comparison_frame = -1;
+		_support_comparison_frame = -1;
 		_source_player_index = -1;
 	}
 };

code/network/multi_obj.cpp

@@ -139,8 +139,12 @@ struct oo_general_info {
 
 oo_general_info Oo_info;
 
-// flags
-bool Afterburn_hack = false;			// HACK!!!
+// This is part of a fix for AB trails and related model animations not working
+// for other ships in multi. The "hack" part is so the fix applies to hosts as well.
+// This could possibly be revisited in a future multi bump with a better solution - taylor
+// See: https://github.com/scp-fs2open/fs2open.github.com/commit/186bd01
+// Or: "mantis bug 895 response" in SCP Internal on HLP forums
+static bool Afterburn_hack = false;			// HACK!!!
 
 // returns the last frame's index.
 int multi_find_prev_frame_idx();
@@ -632,10 +636,9 @@ void multi_ship_record_do_rollback()
 
 	// set up all restore points and ship portion of the collision list
 	for (ship& cur_ship : Ships) {
-
-		// once this happens, we've run out of ships.
+		// skip destroyed ships
 		if (cur_ship.objnum < 0) {
-			break;
+			continue;
 		}
 
 		objp = &Objects[cur_ship.objnum];
@@ -904,10 +907,10 @@ void multi_ship_record_signal_update(int objnum, TIMESTAMP lower_time_limit, TIM
 		}
 	}
 
-	if (prev_index < 0 || post_index < 0) { 
-		mprintf(("Getting prev_index %d and post_index %d, which is not valid, while trying to update the ship record.\n", prev_index, post_index));
+	if ((prev_index < 0) || (prev_index == post_index)) {
 		return;
-	} else if (prev_index == post_index) {
+	} else if (post_index < 0) {
+		mprintf(("Getting prev_index %d and post_index %d, which is not valid, while trying to update the ship record.\n", prev_index, post_index));
 		return;
 	}
 
@@ -1838,10 +1841,18 @@ int multi_oo_unpack_data(net_player* pl, ubyte* data, int seq_num, int time_delt
 	// SPECIAL CLIENT INFO
 	// ---------------------------------------------------------------------------------------------------------------
 
+	// make sure the ab hack is reset before we read in new info
+	Afterburn_hack = false;
+
 	// if this is from a player, read his button info
 	if(MULTIPLAYER_MASTER){
 		int r0 = multi_oo_unpack_client_data(pl, data + offset, seq_num > Interp_info[objnum].get_client_info_comparison_frame());
 		offset += r0;
+
+		// update comparison frame
+		if (seq_num > Interp_info[objnum].get_client_info_comparison_frame()) {
+			Interp_info[objnum].set_client_info_comparison_frame(seq_num);
+		}
 	}
 
 	// ---------------------------------------------------------------------------------------------------------------
@@ -2127,15 +2138,6 @@ int multi_oo_unpack_data(net_player* pl, ubyte* data, int seq_num, int time_delt
 
 		if( seq_num > Interp_info[objnum].get_ai_comparison_frame() ){
 			if ( shipp->ai_index >= 0 ){
-				// make sure to undo the wrap if it occurred during compression for unset ai mode.
-				if (umode == 255) {
-					Ai_info[shipp->ai_index].mode = -1; 
-				}
-				else {
-					Ai_info[shipp->ai_index].mode = umode;
-				}
-				Ai_info[shipp->ai_index].submode = submode;		
-
 				// set this guy's target objnum, and other info
 				target_objp = multi_get_network_object( target_signature );
 
@@ -2182,29 +2184,30 @@ int multi_oo_unpack_data(net_player* pl, ubyte* data, int seq_num, int time_delt
 		GET_INT(ai_submode);
 		GET_USHORT(dock_sig);		
 
-		// verify that it's a valid ship							
-		if((shipp != nullptr) && (shipp->ai_index >= 0) && (shipp->ai_index < MAX_AI_INFO)){
-			// bash ai info, this info does not get rebashed, because it is not as vital.
-			Ai_info[shipp->ai_index].ai_flags.from_u64(ai_flags);
-			Ai_info[shipp->ai_index].mode = ai_mode;
-			Ai_info[shipp->ai_index].submode = ai_submode;
-
-			object *objp = multi_get_network_object( dock_sig );
-			if(objp != nullptr){
-				Ai_info[shipp->ai_index].support_ship_objnum = OBJ_INDEX(objp);
-				if ((objp->instance > -1) && (objp->type == OBJ_SHIP)) {
-					Ai_info[shipp->ai_index].goals[0].target_name = Ships[objp->instance].ship_name;
-					Ai_info[shipp->ai_index].goals[0].target_signature = objp->signature;
-				} else {
-					Ai_info[shipp->ai_index].goals[0].target_name = nullptr;
-					Ai_info[shipp->ai_index].goals[0].target_signature = 0;
+		if (seq_num > Interp_info[objnum].get_support_comparison_frame()) {
+			// verify that it's a valid ship
+			if((shipp != nullptr) && (shipp->ai_index >= 0) && (shipp->ai_index < MAX_AI_INFO)){
+				// bash ai info, this info does not get rebashed, because it is not as vital.
+				Ai_info[shipp->ai_index].ai_flags.from_u64(ai_flags);
+				Ai_info[shipp->ai_index].mode = ai_mode;
+				Ai_info[shipp->ai_index].submode = ai_submode;
+
+				object *objp = multi_get_network_object( dock_sig );
+				if(objp != nullptr){
+					Ai_info[shipp->ai_index].support_ship_objnum = OBJ_INDEX(objp);
+					if ((objp->instance > -1) && (objp->type == OBJ_SHIP)) {
+						Ai_info[shipp->ai_index].goals[0].target_name = Ships[objp->instance].ship_name;
+						Ai_info[shipp->ai_index].goals[0].target_signature = objp->signature;
+					} else {
+						Ai_info[shipp->ai_index].goals[0].target_name = nullptr;
+						Ai_info[shipp->ai_index].goals[0].target_signature = 0;
+					}
 				}
 			}
-		}			
-	} 
 
-	// make sure the ab hack is reset before we read in new info
-	Afterburn_hack = false;
+			Interp_info[objnum].set_support_comparison_frame(seq_num);
+		}
+	}
 
 	// afterburner info
 	if ( (oo_flags & OO_AFTERBURNER_NEW) || Afterburn_hack ) {

code/network/multimsgs.cpp

@@ -7837,7 +7837,7 @@ void send_non_homing_fired_packet(ship* shipp, int banks_or_number_of_missiles_f
 
 	// We need the time elpased, so send the last frame we got from the server and how much time has happened since then.
 	int last_received_frame = multi_client_lookup_frame_idx();
-	auto time_elapsed = static_cast<ushort>(timestamp_since(multi_client_lookup_frame_timestamp()));
+	auto time_elapsed = static_cast<ushort>(Multi_Timing_Info.get_current_time() - multi_client_lookup_frame_timestamp());
 
 	ADD_INT(last_received_frame);
 	ADD_USHORT(time_elapsed);

code/ship/ship.cpp

@@ -15441,7 +15441,7 @@ int ship_type_name_lookup(const char *name)
 // subsysp is a pointer to the subsystem.
 int get_subsystem_pos(vec3d* pos, const object* objp, const ship_subsys* subsysp)
 {
-	if (subsysp == NULL) {
+	if ( !subsysp || !subsysp->system_info ) {
 		*pos = objp->pos;
 		return 0;
 	}

code/ship/shiphit.cpp

@@ -31,6 +31,7 @@
 #include "mission/missionlog.h"
 #include "mod_table/mod_table.h"
 #include "network/multi.h"
+#include "network/multi_interpolate.h"
 #include "network/multi_pmsg.h"
 #include "network/multi_respawn.h"
 #include "network/multimsgs.h"
@@ -2151,6 +2152,11 @@ void ship_apply_whack(const vec3d *force, const vec3d *hit_pos, object *objp)
 		game_whack_apply( -test.xyz.x, -test.xyz.y );
 	}
 
+	if ((Game_mode & GM_MULTIPLAYER) && (objp->type == OBJ_SHIP)) {
+		// temporarily set this as an uninterpolated ship, to make the collision
+		// look more natural until the next update comes in.
+		Interp_info[OBJ_INDEX(objp)].force_interpolation_mode();
+	}
 
 	if (object_is_docked(objp))
 	{