Proper player inventory and hp modification tracking and sending accordingly

src/player.cpp

@@ -184,13 +184,17 @@ ServerRemotePlayer::ServerRemotePlayer(ServerEnvironment *env):
 	ServerActiveObject(env, v3f(0,0,0)),
 	m_last_good_position(0,0,0),
 	m_last_good_position_age(0),
-	m_additional_items()
+	m_additional_items(),
+	m_inventory_not_sent(false),
+	m_hp_not_sent(false)
 {
 }
 ServerRemotePlayer::ServerRemotePlayer(ServerEnvironment *env, v3f pos_, u16 peer_id_,
 		const char *name_):
 	Player(env->getGameDef()),
-	ServerActiveObject(env, pos_)
+	ServerActiveObject(env, pos_),
+	m_inventory_not_sent(false),
+	m_hp_not_sent(false)
 {
 	setPosition(pos_);
 	peer_id = peer_id_;
@@ -249,6 +253,8 @@ bool ServerRemotePlayer::addToInventory(InventoryItem *item)
 	// Add to inventory
 	InventoryItem *leftover = ilist->addItem(item);
 	assert(!leftover);
+	
+	m_inventory_not_sent = true;
 
 	return true;
 }
@@ -295,9 +301,12 @@ void ServerRemotePlayer::completeAddToInventoryLater(u16 preferred_index)
 		delete leftover;
 	}
 	m_additional_items.clear();
+	m_inventory_not_sent = true;
 }
 void ServerRemotePlayer::setHP(s16 hp_)
 {
+	s16 oldhp = hp;
+
 	hp = hp_;
 
 	// FIXME: don't hardcode maximum HP, make configurable per object
@@ -305,6 +314,9 @@ void ServerRemotePlayer::setHP(s16 hp_)
 		hp = 0;
 	else if(hp > 20)
 		hp = 20;
+
+	if(hp != oldhp)
+		m_hp_not_sent = true;
 }
 s16 ServerRemotePlayer::getHP()
 {

src/player.h

@@ -238,6 +238,8 @@ public:
 	v3f m_last_good_position;
 	float m_last_good_position_age;
 	std::vector<InventoryItem*> m_additional_items;
+	bool m_inventory_not_sent;
+	bool m_hp_not_sent;
 	
 private:
 };

src/server.cpp

@@ -1376,12 +1376,7 @@ void Server::AsyncRunStep()
 	*/
 
 	/*
-		Check player movements
-
-		NOTE: Actually the server should handle player physics like the
-		client does and compare player's position to what is calculated
-		on our side. This is required when eg. players fly due to an
-		explosion.
+		Handle players
 	*/
 	{
 		JMutexAutoLock lock(m_env_mutex);
@@ -1404,6 +1399,15 @@ void Server::AsyncRunStep()
 					(m_env->getPlayer(client->peer_id));
 			if(player==NULL)
 				continue;
+			
+			/*
+				Check player movements
+
+				NOTE: Actually the server should handle player physics like the
+				client does and compare player's position to what is calculated
+				on our side. This is required when eg. players fly due to an
+				explosion.
+			*/
 			player->m_last_good_position_age += dtime;
 			if(player->m_last_good_position_age >= 2.0){
 				float age = player->m_last_good_position_age;
@@ -1425,6 +1429,17 @@ void Server::AsyncRunStep()
 				}
 				player->m_last_good_position_age = 0;
 			}
+
+			/*
+				Send player inventories and HPs if necessary
+			*/
+			if(player->m_inventory_not_sent){
+				UpdateCrafting(player->peer_id);
+				SendInventory(player->peer_id);
+			}
+			if(player->m_hp_not_sent){
+				SendPlayerHP(player);
+			}
 		}
 	}
 	
@@ -3181,6 +3196,8 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
 						{
 							mlist->deleteItem(item_i);
 						}
+
+						srp->m_inventory_not_sent = true;
 					}
 				}
 
@@ -3208,12 +3225,13 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
 				{
 					// Add a item to inventory
 					player->inventory.addItem("main", item);
+					srp->m_inventory_not_sent = true;
 				}
 
 				item = NULL;
 
 				if(mineral != MINERAL_NONE)
-				  item = getDiggedMineralItem(mineral, this);
+					item = getDiggedMineralItem(mineral, this);
 			
 				// If not mineral
 				if(item == NULL)
@@ -3232,6 +3250,7 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
 				{
 					// Add a item to inventory
 					player->inventory.addItem("main", item);
+					srp->m_inventory_not_sent = true;
 				}
 			}
 
@@ -3373,6 +3392,7 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
 							ilist->deleteItem(item_i);
 						else
 							mitem->remove(1);
+						srp->m_inventory_not_sent = true;
 					}
 					
 					/*
@@ -3480,9 +3500,11 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
 					if(remove && g_settings->getBool("creative_mode") == false)
 					{
 						InventoryList *ilist = player->inventory.getList("main");
-						if(ilist)
+						if(ilist){
 							// Remove from inventory and send inventory
 							ilist->deleteItem(item_i);
+							srp->m_inventory_not_sent = true;
+						}
 					}
 				}
 			}
@@ -3534,12 +3556,15 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
 					<<", pointing at "<<pointed.dump()<<std::endl;
 
 			bool remove = item->use(m_env, srp, pointed);
+			
 			if(remove && g_settings->getBool("creative_mode") == false)
 			{
 				InventoryList *ilist = player->inventory.getList("main");
-				if(ilist)
+				if(ilist){
 					// Remove from inventory and send inventory
 					ilist->deleteItem(item_i);
+					srp->m_inventory_not_sent = true;
+				}
 			}
 
 		} // action == 4
@@ -3555,11 +3580,6 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
 
 		// Complete add_to_inventory_later
 		srp->completeAddToInventoryLater(item_i);
-
-		// Send inventory
-		// FIXME: Shouldn't be done unless something changed.
-		UpdateCrafting(player->peer_id);
-		SendInventory(player->peer_id);
 	}
 	else
 	{
@@ -3618,9 +3638,9 @@ void Server::inventoryModified(InventoryContext *c, std::string id)
 	if(id == "current_player")
 	{
 		assert(c->current_player);
-		// Send inventory
-		UpdateCrafting(c->current_player->peer_id);
-		SendInventory(c->current_player->peer_id);
+		ServerRemotePlayer *srp =
+				static_cast<ServerRemotePlayer*>(c->current_player);
+		srp->m_inventory_not_sent = true;
 		return;
 	}
 	
@@ -3912,9 +3932,12 @@ void Server::SendInventory(u16 peer_id)
 {
 	DSTACK(__FUNCTION_NAME);
 	
-	Player* player = m_env->getPlayer(peer_id);
+	ServerRemotePlayer* player =
+			static_cast<ServerRemotePlayer*>(m_env->getPlayer(peer_id));
 	assert(player);
 
+	player->m_inventory_not_sent = false;
+
 	/*
 		Serialize it
 	*/