Don't send ping updates one packet at a time

* Use the batch update feature of the packet.
* Lengthen interval between time and ping update packets (ref. http://github.com/cuberite/cuberite/issues/4082#issuecomment-348675321).

src/Blocks/BroadcastInterface.h

@@ -53,7 +53,7 @@ public:
 	virtual void BroadcastPlayerListRemovePlayer     (const cPlayer & a_Player, const cClientHandle * a_Exclude = nullptr) = 0;
 	virtual void BroadcastPlayerListUpdateDisplayName(const cPlayer & a_Player, const AString & a_CustomName, const cClientHandle * a_Exclude = nullptr) = 0;
 	virtual void BroadcastPlayerListUpdateGameMode   (const cPlayer & a_Player, const cClientHandle * a_Exclude = nullptr) = 0;
-	virtual void BroadcastPlayerListUpdatePing       (const cPlayer & a_Player, const cClientHandle * a_Exclude = nullptr) = 0;
+	virtual void BroadcastPlayerListUpdatePing       () = 0;
 	virtual void BroadcastRemoveEntityEffect         (const cEntity & a_Entity, int a_EffectID, const cClientHandle * a_Exclude = nullptr) = 0;
 	virtual void BroadcastScoreboardObjective        (const AString & a_Name, const AString & a_DisplayName, Byte a_Mode) = 0;
 	virtual void BroadcastScoreUpdate                (const AString & a_Objective, const AString & a_PlayerName, cObjective::Score a_Score, Byte a_Mode) = 0;

src/Broadcaster.cpp

@@ -488,11 +488,11 @@ void cWorld::BroadcastPlayerListUpdateGameMode(const cPlayer & a_Player, const c
 
 
 
-void cWorld::BroadcastPlayerListUpdatePing(const cPlayer & a_Player, const cClientHandle * a_Exclude)
+void cWorld::BroadcastPlayerListUpdatePing()
 {
-	ForClientsInWorld(*this, a_Exclude, [&](cClientHandle & a_Client)
+	ForClientsInWorld(*this, nullptr, [&](cClientHandle & a_Client)
 		{
-			a_Client.SendPlayerListUpdatePing(a_Player);
+			a_Client.SendPlayerListUpdatePing();
 		}
 	);
 }

src/ChunkMap.h

@@ -378,7 +378,7 @@ public:
 	void ChunkValidated(void);  // Called by chunks that have become valid
 
 	/** Returns the CS for locking the chunkmap; only cWorld::cLock may use this function! */
-	cCriticalSection & GetCS(void) { return m_CSChunks; }
+	cCriticalSection & GetCS(void) const { return m_CSChunks; }
 
 	/** Increments (a_AlwaysTicked == true) or decrements (false) the m_AlwaysTicked counter for the specified chunk.
 	If the m_AlwaysTicked counter is greater than zero, the chunk is ticked in the tick-thread regardless of

src/ClientHandle.cpp

@@ -2715,9 +2715,9 @@ void cClientHandle::SendPlayerListUpdateGameMode(const cPlayer & a_Player)
 
 
 
-void cClientHandle::SendPlayerListUpdatePing(const cPlayer & a_Player)
+void cClientHandle::SendPlayerListUpdatePing()
 {
-	m_Protocol->SendPlayerListUpdatePing(a_Player);
+	m_Protocol->SendPlayerListUpdatePing();
 }
 
 

src/ClientHandle.h

@@ -193,7 +193,7 @@ public:  // tolua_export
 	void SendPlayerListRemovePlayer     (const cPlayer & a_Player);
 	void SendPlayerListUpdateDisplayName(const cPlayer & a_Player, const AString & a_CustomName);
 	void SendPlayerListUpdateGameMode   (const cPlayer & a_Player);
-	void SendPlayerListUpdatePing       (const cPlayer & a_Player);
+	void SendPlayerListUpdatePing       ();
 	void SendPlayerMaxSpeed             (void);  ///< Informs the client of the maximum player speed (1.6.1+)
 	void SendPlayerMoveLook             (void);
 	void SendPlayerPosition             (void);

src/Entities/Player.cpp

@@ -34,9 +34,6 @@
 // 6000 ticks or 5 minutes
 #define PLAYER_INVENTORY_SAVE_INTERVAL 6000
 
-// 1000 = once per second
-#define PLAYER_LIST_TIME_MS std::chrono::milliseconds(1000)
-
 namespace
 {
 
@@ -131,8 +128,6 @@ cPlayer::cPlayer(const cClientHandlePtr & a_Client) :
 	SetMaxHealth(MAX_HEALTH);
 	m_Health = MAX_HEALTH;
 
-	m_LastPlayerListTime = std::chrono::steady_clock::now();
-
 	cWorld * World = nullptr;
 	if (!LoadFromDisk(World))
 	{
@@ -3205,13 +3200,6 @@ void cPlayer::Tick(std::chrono::milliseconds a_Dt, cChunk & a_Chunk)
 	// Update items (e.g. Maps)
 	m_Inventory.UpdateItems();
 
-	// Send Player List (Once per m_LastPlayerListTime/1000 ms)
-	if (m_LastPlayerListTime + PLAYER_LIST_TIME_MS <= std::chrono::steady_clock::now())
-	{
-		m_World->BroadcastPlayerListUpdatePing(*this);
-		m_LastPlayerListTime = std::chrono::steady_clock::now();
-	}
-
 	if (m_TicksUntilNextSave == 0)
 	{
 		SaveToDisk();

src/Entities/Player.h

@@ -656,8 +656,6 @@ private:
 	/** The item being dragged by the cursor while in a UI window */
 	cItem m_DraggingItem;
 
-	std::chrono::steady_clock::time_point m_LastPlayerListTime;
-
 	cClientHandlePtr m_ClientHandle;
 
 	cSlotNums m_InventoryPaintSlots;

src/Protocol/Protocol.h

@@ -410,7 +410,7 @@ public:
 	virtual void SendPlayerListHeaderFooter     (const cCompositeChat & a_Header, const cCompositeChat & a_Footer) = 0;
 	virtual void SendPlayerListRemovePlayer     (const cPlayer & a_Player) = 0;
 	virtual void SendPlayerListUpdateGameMode   (const cPlayer & a_Player) = 0;
-	virtual void SendPlayerListUpdatePing       (const cPlayer & a_Player) = 0;
+	virtual void SendPlayerListUpdatePing       () = 0;
 	virtual void SendPlayerListUpdateDisplayName(const cPlayer & a_Player, const AString & a_CustomName) = 0;
 	virtual void SendPlayerMaxSpeed             (void) = 0;  ///< Informs the client of the maximum player speed (1.6.1+)
 	virtual void SendPlayerMoveLook             (void) = 0;

src/Protocol/Protocol_1_8.cpp

@@ -1111,15 +1111,21 @@ void cProtocol_1_8_0::SendPlayerListUpdateGameMode(const cPlayer & a_Player)
 
 
 
-void cProtocol_1_8_0::SendPlayerListUpdatePing(const cPlayer & a_Player)
+void cProtocol_1_8_0::SendPlayerListUpdatePing()
 {
 	ASSERT(m_State == 3);  // In game mode?
 
 	cPacketizer Pkt(*this, pktPlayerList);
 	Pkt.WriteVarInt32(2);
-	Pkt.WriteVarInt32(1);
+
-	Pkt.WriteUUID(a_Player.GetUUID());
+	const auto World = m_Client->GetPlayer()->GetWorld();
-	Pkt.WriteVarInt32(static_cast<UInt32>(a_Player.GetClientHandle()->GetPing()));
+	Pkt.WriteVarInt32(static_cast<UInt32>(World->GetPlayerCount()));
+	World->ForEachPlayer([&Pkt](cPlayer & a_Player)
+	{
+		Pkt.WriteUUID(a_Player.GetUUID());
+		Pkt.WriteVarInt32(static_cast<UInt32>(a_Player.GetClientHandle()->GetPing()));
+		return false;
+	});
 }
 
 

src/Protocol/Protocol_1_8.h

@@ -94,7 +94,7 @@ public:
 	virtual void SendPlayerListRemovePlayer     (const cPlayer & a_Player) override;
 	virtual void SendPlayerListUpdateDisplayName(const cPlayer & a_Player, const AString & a_CustomName) override;
 	virtual void SendPlayerListUpdateGameMode   (const cPlayer & a_Player) override;
-	virtual void SendPlayerListUpdatePing       (const cPlayer & a_Player) override;
+	virtual void SendPlayerListUpdatePing       () override;
 	virtual void SendPlayerMaxSpeed             (void) override;
 	virtual void SendPlayerMoveLook             (void) override;
 	virtual void SendPlayerPosition             (void) override;

src/World.cpp

@@ -91,7 +91,7 @@ namespace World
 ////////////////////////////////////////////////////////////////////////////////
 // cWorld::cLock:
 
-cWorld::cLock::cLock(cWorld & a_World) :
+cWorld::cLock::cLock(const cWorld & a_World) :
 	Super(&(a_World.m_ChunkMap.GetCS()))
 {
 }
@@ -168,7 +168,7 @@ cWorld::cWorld(
 	m_IsDaylightCycleEnabled(true),
 	m_WorldAge(0),
 	m_TimeOfDay(0),
-	m_LastTimeUpdate(0),
+	m_WorldTickAge(0),
 	m_LastChunkCheck(0),
 	m_LastSave(0),
 	m_SkyDarkness(0),
@@ -959,33 +959,38 @@ void cWorld::Stop(cDeadlockDetect & a_DeadlockDetect)
 
 void cWorld::Tick(std::chrono::milliseconds a_Dt, std::chrono::milliseconds a_LastTickDurationMSec)
 {
-	// Call the plugins
+	// Notify the plugins:
 	cPluginManager::Get()->CallHookWorldTick(*this, a_Dt, a_LastTickDurationMSec);
 
 	m_WorldAge += a_Dt;
+	m_WorldTickAge += 1;
 
 	if (m_IsDaylightCycleEnabled)
 	{
-		// We need sub-tick precision here, that's why we store the time in milliseconds and calculate ticks off of it
 		m_TimeOfDay += a_Dt;
 
-		// Wrap time of day each 20 minutes (1200 seconds)
+		// Wrap time of day every 20 minutes (1200 seconds):
 		if (m_TimeOfDay > std::chrono::minutes(20))
 		{
 			m_TimeOfDay -= std::chrono::minutes(20);
 		}
 
-		// Updates the sky darkness based on current time of day
+		// Updates the sky darkness based on current time of day:
 		UpdateSkyDarkness();
 
-		// Broadcast time update every 40 ticks (2 seconds)
+		// Broadcast time update every 64 ticks (3.2 seconds):
-		if (m_LastTimeUpdate < m_WorldAge - cTickTime(40))
+		if ((m_WorldTickAge % 64) == 0)
 		{
 			BroadcastTimeUpdate();
-			m_LastTimeUpdate = std::chrono::duration_cast<cTickTimeLong>(m_WorldAge);
 		}
 	}
 
+	// Broadcast player list pings every 256 ticks (12.8 seconds):
+	if ((m_WorldTickAge % 256) == 0)
+	{
+		BroadcastPlayerListUpdatePing();
+	}
+
 	TickQueuedChunkDataSets();
 	TickQueuedBlocks();
 	m_ChunkMap.Tick(a_Dt);
@@ -993,11 +998,10 @@ void cWorld::Tick(std::chrono::milliseconds a_Dt, std::chrono::milliseconds a_La
 	TickQueuedEntityAdditions();
 	m_MapManager.TickMaps();
 	TickQueuedTasks();
+	TickWeather(static_cast<float>(a_Dt.count()));
 
 	GetSimulatorManager()->Simulate(static_cast<float>(a_Dt.count()));
 
-	TickWeather(static_cast<float>(a_Dt.count()));
-
 	if (m_WorldAge - m_LastChunkCheck > std::chrono::seconds(10))
 	{
 		// Unload every 10 seconds
@@ -2535,6 +2539,16 @@ bool cWorld::ForEachEntityInBox(const cBoundingBox & a_Box, cEntityCallback a_Ca
 
 
 
+size_t cWorld::GetPlayerCount() const
+{
+	cLock Lock(*this);
+	return m_Players.size();
+}
+
+
+
+
+
 bool cWorld::DoWithEntityByID(UInt32 a_UniqueID, cEntityCallback a_Callback)
 {
 	// First check the entities-to-add:

src/World.h

@@ -78,7 +78,7 @@ public:
 	{
 		using Super = cCSLock;
 	public:
-		cLock(cWorld & a_World);
+		cLock(const cWorld & a_World);
 	};
 
 
@@ -212,7 +212,7 @@ public:
 	virtual void BroadcastPlayerListRemovePlayer     (const cPlayer & a_Player, const cClientHandle * a_Exclude = nullptr) override;
 	virtual void BroadcastPlayerListUpdateDisplayName(const cPlayer & a_Player, const AString & a_CustomName, const cClientHandle * a_Exclude = nullptr) override;
 	virtual void BroadcastPlayerListUpdateGameMode   (const cPlayer & a_Player, const cClientHandle * a_Exclude = nullptr) override;
-	virtual void BroadcastPlayerListUpdatePing       (const cPlayer & a_Player, const cClientHandle * a_Exclude = nullptr) override;
+	virtual void BroadcastPlayerListUpdatePing       () override;
 	virtual void BroadcastRemoveEntityEffect         (const cEntity & a_Entity, int a_EffectID, const cClientHandle * a_Exclude = nullptr) override;
 	virtual void BroadcastScoreboardObjective        (const AString & a_Name, const AString & a_DisplayName, Byte a_Mode) override;
 	virtual void BroadcastScoreUpdate                (const AString & a_Objective, const AString & a_Player, cObjective::Score a_Score, Byte a_Mode) override;
@@ -303,6 +303,9 @@ public:
 	If any chunk in the box is missing, ignores the entities in that chunk silently. */
 	virtual bool ForEachEntityInBox(const cBoundingBox & a_Box, cEntityCallback a_Callback) override;  // Exported in ManualBindings.cpp
 
+	/** Returns the number of players currently in this world. */
+	size_t GetPlayerCount() const;
+
 	/** Calls the callback if the entity with the specified ID is found, with the entity object as the callback param.
 	Returns true if entity found and callback returned false. */
 	bool DoWithEntityByID(UInt32 a_UniqueID, cEntityCallback a_Callback);  // Exported in ManualBindings.cpp
@@ -1054,11 +1057,20 @@ private:
 	bool   m_IsDaylightCycleEnabled;
 
 	/** The age of the world.
-	Monotonic, always increasing each game tick, persistent across server restart. */
+	Monotonic, always increasing each game tick, persistent across server restart.
+	We need sub-tick precision here, that's why we store the time in milliseconds and calculate ticks off of it. */
 	std::chrono::milliseconds m_WorldAge;
 
+	/** The duration of one Minecraft day that has elapsed.
+	Wraps every 20 minutes.
+	We need sub-tick precision here, that's why we store the time in milliseconds and calculate ticks off of it. */
 	std::chrono::milliseconds  m_TimeOfDay;
-	cTickTimeLong  m_LastTimeUpdate;    // The tick in which the last time update has been sent.
+
+	/** The age of the world, in ticks.
+	Monotonic, but does not persist across restarts.
+	Used for less important but heavy tasks that run periodically. These tasks don't need to follow wallclock time, and slowing their rate down if TPS drops is desirable. */
+	unsigned long long m_WorldTickAge;
+
 	cTickTimeLong  m_LastChunkCheck;        // The last WorldAge (in ticks) in which unloading and possibly saving was triggered
 	cTickTimeLong  m_LastSave;          // The last WorldAge (in ticks) in which save-all was triggerred
 	std::map<cMonster::eFamily, cTickTimeLong> m_LastSpawnMonster;  // The last WorldAge (in ticks) in which a monster was spawned (for each megatype of monster)  // MG TODO : find a way to optimize without creating unmaintenability (if mob IDs are becoming unrowed)