Merge pull request #7306 from notimaginative/multi_interp_fixes

Collection of fixes for multiple issues with the multi code found by Claude AI (by way of Goober5000), plus additional issues noted by myself while checking the code. These fixes are primarily focused on the interpolation code.

These changes should not be breaking and are largely exclusive to server-side handling, but still need to be tested thoroughly before being merged. Any tests must be done with at least the server (master) using the fixes.

Author: wookieejedi

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;
 	}
 
@@ -1840,10 +1843,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);
+		}
 	}
 
 	// ---------------------------------------------------------------------------------------------------------------
@@ -2129,15 +2140,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 );
 
@@ -2184,29 +2186,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/multi_options.cpp

@@ -318,6 +318,19 @@ void multi_options_read_config()
 							Multi_options_g.webapiPort = (ushort) result;
 						}
 					}
+				} else
+				// set framerate for standalone
+				if ( SETTING("+fps_cap") ) {
+					NEXT_TOKEN();
+					if (tok != nullptr) {
+						if ((atoi(tok) >= 15) && (atoi(tok) <= 120)) {
+							Multi_options_g.std_framecap = atoi(tok);
+						}
+					}
+				} else
+				// disable rollback for primary/dumbfire weapons
+				if ( SETTING("+rollback_disabled") ) {
+					Multi_options_g.std_rollback = false;
 				}
 			}
 

code/network/multi_options.h

@@ -50,6 +50,7 @@ typedef struct multi_global_options {
 	char		std_passwd[STD_PASSWD_LEN+1];							// standalone host password
 	char		std_pname[STD_NAME_LEN+1];								// permanent name for the standalone - if any
 	int		std_framecap;												// standalone frame cap
+	bool	std_rollback;												// use rollback for primary/dumbfire shots
 
 	ushort		webapiPort;
 	SCP_string	webapiUsername;
@@ -79,6 +80,7 @@ typedef struct multi_global_options {
 		memset(std_passwd, 0, STD_PASSWD_LEN+1);
 		memset(std_pname, 0, STD_NAME_LEN+1);
 		std_framecap = 30;
+		std_rollback = true;
 
 		webapiPort = 8080;
 		webapiUsername = "admin";

code/network/multimsgs.cpp

@@ -7845,7 +7845,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);
@@ -7939,7 +7939,7 @@ void process_non_homing_fired_packet(ubyte* data, header* hinfo)
 
 	object* objp_ref = multi_get_network_object(target_ref);
 
-	if (objp_ref == nullptr || objp_ref->type != OBJ_SHIP) {
+	if ((Is_standalone && !Multi_options_g.std_rollback) || !objp_ref || (objp_ref->type != OBJ_SHIP)) {
 		// new way failed, use the old new way.
 
 		if (objp_ref != nullptr){

code/ship/ship.cpp

@@ -15449,7 +15449,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))
 	{