Client & ClientEnvirnment: don't create fake events (#5676)

nerzhul · web-flow · commit 3db66b453152 · 2017-04-29T17:25:25.000+02:00
Instead of create fake events on the stack on each loop call (Game::run), verify is queue is empty or not and handle event directly if there is.

This prevents fake ClientEvent creation &amp; memory allocations

Same fix is also applied on ClientEnvironment, &amp; rename getClientEvent to getClientEnvEvent to match ClientEnvEvent object
diff --git a/src/client.cpp b/src/client.cpp
@@ -411,16 +411,14 @@ void Client::step(float dtime)
 	/*
 		Get events
 	*/
-	for(;;) {
-		ClientEnvEvent event = m_env.getClientEvent();
-		if(event.type == CEE_NONE) {
-			break;
-		}
-		else if(event.type == CEE_PLAYER_DAMAGE) {
-			if(m_ignore_damage_timer <= 0) {
-				u8 damage = event.player_damage.amount;
+	while (m_env.hasClientEnvEvents()) {
+		ClientEnvEvent envEvent = m_env.getClientEnvEvent();
+
+		if (envEvent.type == CEE_PLAYER_DAMAGE) {
+			if (m_ignore_damage_timer <= 0) {
+				u8 damage = envEvent.player_damage.amount;
 
-				if(event.player_damage.send_to_server)
+				if (envEvent.player_damage.send_to_server)
 					sendDamage(damage);
 
 				// Add to ClientEvent queue
@@ -431,8 +429,8 @@ void Client::step(float dtime)
 			}
 		}
 		// Protocol v29 or greater obsoleted this event
-		else if (event.type == CEE_PLAYER_BREATH && m_proto_ver < 29) {
-			u16 breath = event.player_breath.amount;
+		else if (envEvent.type == CEE_PLAYER_BREATH && m_proto_ver < 29) {
+			u16 breath = envEvent.player_breath.amount;
 			sendBreath(breath);
 		}
 	}
@@ -1596,14 +1594,11 @@ void Client::addUpdateMeshTaskForNode(v3s16 nodepos, bool ack_to_server, bool ur
 
 ClientEvent Client::getClientEvent()
 {
-	ClientEvent event;
-	if (m_client_event_queue.empty()) {
-		event.type = CE_NONE;
-	}
-	else {
-		event = m_client_event_queue.front();
-		m_client_event_queue.pop();
-	}
+	FATAL_ERROR_IF(m_client_event_queue.empty(),
+			"Cannot getClientEvent, queue is empty.");
+
+	ClientEvent event = m_client_event_queue.front();
+	m_client_event_queue.pop();
 	return event;
 }
 
diff --git a/src/client.h b/src/client.h
@@ -414,7 +414,8 @@ class Client : public con::PeerHandler, public InventoryManager, public IGameDef
 	void updateCameraOffset(v3s16 camera_offset)
 	{ m_mesh_update_thread.m_camera_offset = camera_offset; }
 
-	// Get event from queue. CE_NONE is returned if queue is empty.
+	bool hasClientEvents() const { return !m_client_event_queue.empty(); }
+	// Get event from queue. If queue is empty, it triggers an assertion failure.
 	ClientEvent getClientEvent();
 
 	bool accessDenied() const { return m_access_denied; }
diff --git a/src/clientenvironment.cpp b/src/clientenvironment.cpp
@@ -598,15 +598,13 @@ void ClientEnvironment::getActiveObjects(v3f origin, f32 max_d,
 	}
 }
 
-ClientEnvEvent ClientEnvironment::getClientEvent()
+ClientEnvEvent ClientEnvironment::getClientEnvEvent()
 {
-	ClientEnvEvent event;
-	if(m_client_event_queue.empty())
-		event.type = CEE_NONE;
-	else {
-		event = m_client_event_queue.front();
-		m_client_event_queue.pop();
-	}
+	FATAL_ERROR_IF(m_client_event_queue.empty(),
+			"ClientEnvironment::getClientEnvEvent(): queue is empty");
+
+	ClientEnvEvent event = m_client_event_queue.front();
+	m_client_event_queue.pop();
 	return event;
 }
 
diff --git a/src/clientenvironment.h b/src/clientenvironment.h
@@ -126,8 +126,9 @@ class ClientEnvironment : public Environment
 	void getActiveObjects(v3f origin, f32 max_d,
 		std::vector<DistanceSortedActiveObject> &dest);
 
-	// Get event from queue. CEE_NONE is returned if queue is empty.
-	ClientEnvEvent getClientEvent();
+	bool hasClientEnvEvents() const { return !m_client_event_queue.empty(); }
+	// Get event from queue. If queue is empty, it triggers an assertion failure.
+	ClientEnvEvent getClientEnvEvent();
 
 	/*!
 	 * Gets closest object pointed by the shootline.
diff --git a/src/game.cpp b/src/game.cpp
@@ -3055,11 +3055,10 @@ inline void Game::step(f32 *dtime)
 
 void Game::processClientEvents(CameraOrientation *cam)
 {
-	ClientEvent event = client->getClientEvent();
-
 	LocalPlayer *player = client->getEnv().getLocalPlayer();
 
-	for ( ; event.type != CE_NONE; event = client->getClientEvent()) {
+	while (client->hasClientEvents()) {
+		ClientEvent event = client->getClientEvent();
 
 		switch (event.type) {
 		case CE_PLAYER_DAMAGE:

Original file line number	Diff line number	Diff line change
`@@ -598,15 +598,13 @@ void ClientEnvironment::getActiveObjects(v3f origin, f32 max_d,`
`598`	`598`	`}`
`599`	`599`	`}`
`600`	`600`
`601`		`-ClientEnvEvent ClientEnvironment::getClientEvent()`
	`601`	`+ClientEnvEvent ClientEnvironment::getClientEnvEvent()`
`602`	`602`	`{`
`603`		`- ClientEnvEvent event;`
`604`		`- if(m_client_event_queue.empty())`
`605`		`- event.type = CEE_NONE;`
`606`		`- else {`
`607`		`- event = m_client_event_queue.front();`
`608`		`- m_client_event_queue.pop();`
`609`		`- }`
	`603`	`+ FATAL_ERROR_IF(m_client_event_queue.empty(),`
	`604`	`+ "ClientEnvironment::getClientEnvEvent(): queue is empty");`
	`605`	`+`
	`606`	`+ ClientEnvEvent event = m_client_event_queue.front();`
	`607`	`+ m_client_event_queue.pop();`
`610`	`608`	`return event;`
`611`	`609`	`}`
`612`	`610`