From: bones_was_here <bones_was_here@xonotic.au>
Date: Fri, 26 Jul 2024 10:36:27 +0000 (+1000)
Subject: cl_net: fix two subtle flaws and simplify logic
X-Git-Url: https://git.rm.cloudns.org/?a=commitdiff_plain;h=b05cfcb87addc424cc3ac654a61ef8dc5d563ea5;p=xonotic%2Fdarkplaces.git

cl_net: fix two subtle flaws and simplify logic

The code introduced in 42e86ab7892c3d8f5373402a714b73b2e040d81b had two
separate modes which was more complex than necessary and less robust in
high ping high jitter scenarios.

The 0ms packet check didn't work when the cause was aggressive time
synchronisation and a ping spike.

Signed-off-by: bones_was_here <bones_was_here@xonotic.au>
---

diff --git a/cl_input.c b/cl_input.c
index 5468a4e2..eb3118b0 100644
--- a/cl_input.c
+++ b/cl_input.c
@@ -1777,7 +1777,7 @@ void CL_SendMove(void)
 	usercmd_t *cmd;
 	sizebuf_t buf;
 	unsigned char data[1024];
-	float packettime, sv_frametime, lag, frames_per_tic;
+	float packettime, lag;
 	qbool opportune_moment;
 	qbool quemove;
 	qbool important;
@@ -1908,52 +1908,38 @@ void CL_SendMove(void)
 	// (otherwise it is only for prediction)
 
 	// do not send 0ms packets because they mess up physics
-	if(cl.cmd.msec == 0 && cl.time > cl.oldtime && (cls.protocol == PROTOCOL_QUAKEWORLD || cls.signon == SIGNONS))
+	// DP servers discard (treat like lost) predicted moves shorter than 0.0005s
+	// the time advancing check must be unaffected by time sync as it may have caused the short move
+	if(cl.cmd.msec == 0 && cl.mtime[0] > cl.mtime[1] && (cls.protocol == PROTOCOL_QUAKEWORLD || cls.signon == SIGNONS))
 		return;
 
 	// don't send too often or else network connections can get clogged by a
 	// high renderer framerate
 	packettime = 1.0f / bound(10.0f, cl_netfps.value, 1000.0f);
 	if (cl.movevars_ticrate)
-	{
 		packettime = bound(cl.movevars_ticrate * 0.5f, packettime, cl.movevars_ticrate);
-		sv_frametime = cl.movevars_ticrate;
-	}
-	else // fallback, may be affected by server->client network problems
-		sv_frametime = (cl.mtime[0] - cl.mtime[1]) / cl.movevars_timescale;
 
 	// always send if buttons changed or an impulse is pending
 	// even if it violates the rate limit!
 	important = (cl.cmd.impulse || (cl_netimmediatebuttons.integer && cl.cmd.buttons != cl.movecmd[1].buttons));
 
+	// improve and stabilise ping by synchronising with the server
 	lag = cl.mtime[0] - cl.cmd.time;
-	frames_per_tic = sv_frametime / cl.realframetime;
-//	opportune_moment = lag <= cl.realframetime * 2; // FAIL: can miss the moment with uncapped fps
-//	opportune_moment = lag <= cl.realframetime * frames_per_tic * 0.5; // FAIL: too early at high fps, reducing multi causes misses at moderate fps
-	opportune_moment = lag <= cl.realframetime * (frames_per_tic <= 1 ? 1 : sqrt(frames_per_tic)); // perfect
+	//    unknown ticrate    ||     PL or ping spike      || loading
+	if (!cl.movevars_ticrate || lag > cl.movevars_ticrate || lag < 0)
+		opportune_moment = false;
+	else // sync should be possible
+	{
+		float frames_per_tic = cl.movevars_ticrate / cl.realframetime;
+		opportune_moment = lag < 0.999f * (float)cl.realframetime * (frames_per_tic <= 1 ? 1 : sqrtf(frames_per_tic));
+	}
 
-	// Two methods for deciding when to send
-	if (!important)
+	// don't send too often (cl_netfps)
+	if (!important && cl.timesincepacket < packettime * 0.999f
+	&& (!opportune_moment || cl.opt_inputs_since_update))
 	{
-		// Traditional time interval, now used as fallback
-		if (sv_frametime <= 0 || lag > sv_frametime || lag < 0) // unknown ticrate || lag/PL || still connecting
-		{
-			if (cl.timesincepacket < packettime * 0.99999f)
-			{
-//				Con_Printf("^6moveft %f realft %f lag %f tic %f inputsince %d\n", cl.cmd.frametime, cl.realframetime, lag, sv_frametime, cl.opt_inputs_since_update);
-				return;
-			}
-		}
-		// Server-synchronised, for better pings
-		// Sends at least once per server frame
-		else // cl.opt_inputs_since_update is usable
-		{
-			if (!opportune_moment || cl.opt_inputs_since_update >= sv_frametime / packettime)
-			{
-//				Con_Printf("^1moveft %f realft %f lag %f tic %f inputsince %d\n", cl.cmd.frametime, cl.realframetime, lag, sv_frametime, cl.opt_inputs_since_update);
-				return;
-			}
-		}
+//		Con_Printf("^1moveft %f realft %f lag %f tic %f inputsince %d opp %d\n", cl.cmd.frametime, cl.realframetime, lag, cl.movevars_ticrate, cl.opt_inputs_since_update, opportune_moment);
+		return;
 	}
 
 	// don't choke the connection with packets (obey rate limit)
@@ -1964,12 +1950,10 @@ void CL_SendMove(void)
 	if (!NetConn_CanSend(cls.netcon) && !important)
 		return;
 
-//	Con_Printf("%smoveft %f realft %f lag %f tic %f inputsince %d important %d\n", (lag < 0.0005 || !opportune_moment) ? "^3" : "^2", cl.cmd.frametime, cl.realframetime, lag, sv_frametime, cl.opt_inputs_since_update, important);
+//	Con_Printf("%smoveft %f realft %f lag %f tic %f inputsince %d opp %d import %d\n", (lag < 0.0005 || !opportune_moment) ? "^3" : "^2", cl.cmd.frametime, cl.realframetime, lag, cl.movevars_ticrate, cl.opt_inputs_since_update, opportune_moment, important);
 
-	if (opportune_moment) // this check is needed for optimal timing especially with cl_netimmediatebuttons
+	if (opportune_moment)
 		++cl.opt_inputs_since_update;
-
-	// reset the packet timing accumulator
 	cl.timesincepacket = 0;
 
 	buf.maxsize = sizeof(data);