From 5d8a22066c8c1086f7d99180b236f0a669362594 Mon Sep 17 00:00:00 2001
From: sfan5 <sfan5@live.de>
Date: Mon, 26 Feb 2024 20:46:57 +0100
Subject: [PATCH] Change how max_lag is calculated and reported (#14378)

-Change how max_lag is calculated and reported

- Cap singleplayer step at 60Hz

- Clarify dedicated_server_step
---
 builtin/settingtypes.txt |  7 ++++---
 src/client/game.cpp      | 10 ++++++++--
 src/server.cpp           | 28 +++++++++++++++++++++-------
 src/server.h             |  8 ++------
 src/serverenvironment.h  |  2 +-
 src/util/numeric.h       | 10 ++++------
 6 files changed, 40 insertions(+), 25 deletions(-)

diff --git a/builtin/settingtypes.txt b/builtin/settingtypes.txt
index 474e56d6f..cecf3d963 100644
--- a/builtin/settingtypes.txt
+++ b/builtin/settingtypes.txt
@@ -2019,9 +2019,10 @@ ask_reconnect_on_crash (Ask to reconnect after crash) bool false
 
 [**Server/Env Performance]
 
-#    Length of a server tick and the interval at which objects are generally updated over
-#    network, stated in seconds.
-dedicated_server_step (Dedicated server step) float 0.09 0.0
+#    Length of a server tick (the interval at which everything is generally updated),
+#    stated in seconds.
+#    Does not apply to sessions hosted from the client menu.
+dedicated_server_step (Dedicated server step) float 0.09 0.0 1.0
 
 #    Whether players are shown to clients without any range limit.
 #    Deprecated, use the setting player_transfer_distance instead.
diff --git a/src/client/game.cpp b/src/client/game.cpp
index 761484c5c..3f83b0278 100644
--- a/src/client/game.cpp
+++ b/src/client/game.cpp
@@ -2751,10 +2751,16 @@ inline void Game::step(f32 dtime)
 				g_settings->getFloat("fps_max_unfocused") :
 				g_settings->getFloat("fps_max");
 		fps_max = std::max(fps_max, 1.0f);
-		float steplen = 1.0f / fps_max;
+		/*
+		 * Unless you have a barebones game, running the server at more than 60Hz
+		 * is hardly realistic and you're at the point of diminishing returns.
+		 * fps_max is also not necessarily anywhere near the FPS actually achieved
+		 * (also due to vsync).
+		 */
+		fps_max = std::min(fps_max, 60.0f);
 
 		server->setStepSettings(Server::StepSettings{
-				steplen,
+				1.0f / fps_max,
 				m_is_paused
 			});
 
diff --git a/src/server.cpp b/src/server.cpp
index 66f485573..9441f5050 100644
--- a/src/server.cpp
+++ b/src/server.cpp
@@ -646,15 +646,29 @@ void Server::AsyncRunStep(float dtime, bool initial_step)
 
 	{
 		MutexAutoLock lock(m_env_mutex);
-		// Figure out and report maximum lag to environment
 		float max_lag = m_env->getMaxLagEstimate();
-		max_lag *= 0.9998; // Decrease slowly (about half per 5 minutes)
-		if(dtime > max_lag){
-			if(dtime > 0.1 && dtime > max_lag * 2.0)
-				infostream<<"Server: Maximum lag peaked to "<<dtime
-						<<" s"<<std::endl;
-			max_lag = dtime;
+		constexpr float lag_warn_threshold = 2.0f;
+
+		// Decrease value gradually, halve it every minute.
+		if (m_max_lag_decrease.step(dtime, 0.5f)) {
+			// To reproduce this number enter "solve (x)**(60/0.5) = 0.5"
+			// into Wolfram Alpha.
+			max_lag *= 0.99425f;
 		}
+
+		// Report a 20% change to the log but only if we're >10% off target
+		// also report if we crossed into the warning boundary
+		if (dtime >= max_lag * 1.2f ||
+			(max_lag < lag_warn_threshold && dtime >= lag_warn_threshold)) {
+			const float steplen = getStepSettings().steplen;
+			if (dtime > steplen * 1.1f) {
+				auto &to = dtime >= lag_warn_threshold ? warningstream : infostream;
+				to << "Server: Maximum lag peaked at " << dtime
+					<< " (steplen=" << steplen << ")" << std::endl;
+			}
+		}
+		max_lag = std::max(max_lag, dtime),
+
 		m_env->reportMaxLagEstimate(max_lag);
 
 		// Step environment
diff --git a/src/server.h b/src/server.h
index aeb18e592..d262ca3c2 100644
--- a/src/server.h
+++ b/src/server.h
@@ -609,12 +609,14 @@ private:
 	MutexedVariable<std::string> m_async_fatal_error;
 
 	// Some timers
+	float m_time_of_day_send_timer = 0.0f;
 	float m_liquid_transform_timer = 0.0f;
 	float m_liquid_transform_every = 1.0f;
 	float m_masterserver_timer = 0.0f;
 	float m_emergethread_trigger_timer = 0.0f;
 	float m_savemap_timer = 0.0f;
 	IntervalLimiter m_map_timer_and_unload_interval;
+	IntervalLimiter m_max_lag_decrease;
 
 	// Environment
 	ServerEnvironment *m_env = nullptr;
@@ -662,12 +664,6 @@ private:
 	// The server mainly operates in this thread
 	ServerThread *m_thread = nullptr;
 
-	/*
-		Time related stuff
-	*/
-	// Timer for sending time of day over network
-	float m_time_of_day_send_timer = 0.0f;
-
 	/*
 	 	Client interface
 	*/
diff --git a/src/serverenvironment.h b/src/serverenvironment.h
index 5f38827f7..01f232c8c 100644
--- a/src/serverenvironment.h
+++ b/src/serverenvironment.h
@@ -367,7 +367,7 @@ public:
 	u32 getGameTime() const { return m_game_time; }
 
 	void reportMaxLagEstimate(float f) { m_max_lag_estimate = f; }
-	float getMaxLagEstimate() { return m_max_lag_estimate; }
+	float getMaxLagEstimate() const { return m_max_lag_estimate; }
 
 	std::set<v3s16>* getForceloadedBlocks() { return &m_active_blocks.m_forceloaded_list; }
 
diff --git a/src/util/numeric.h b/src/util/numeric.h
index d67dd0f5f..ec3145734 100644
--- a/src/util/numeric.h
+++ b/src/util/numeric.h
@@ -350,12 +350,10 @@ class IntervalLimiter
 public:
 	IntervalLimiter() = default;
 
-	/*
-		dtime: time from last call to this method
-		wanted_interval: interval wanted
-		return value:
-			true: action should be skipped
-			false: action should be done
+	/**
+		@param dtime time from last call to this method
+		@param wanted_interval interval wanted
+		@return true if action should be done
 	*/
 	bool step(float dtime, float wanted_interval)
 	{