Fix some common SAO methods to not generate useless update packets

src/inventory.cpp

@@ -601,8 +601,10 @@ ItemStack InventoryList::changeItem(u32 i, const ItemStack &newitem)
 		return newitem;
 
 	ItemStack olditem = m_items[i];
-	m_items[i] = newitem;
-	setModified();
+	if (olditem != newitem) {
+		m_items[i] = newitem;
+		setModified();
+	}
 	return olditem;
 }
 

src/object_properties.h

@@ -20,6 +20,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #pragma once
 
 #include <optional>
+#include <tuple>
 #include <string>
 #include "irrlichttypes_bloated.h"
 #include <iostream>
@@ -76,6 +77,29 @@ struct ObjectProperties
 
 	std::string dump() const;
 
+private:
+	auto tie() const {
+		// Make sure to add new members to this list!
+		return std::tie(
+		textures, colors, collisionbox, selectionbox, visual, mesh, damage_texture_modifier,
+		nametag, infotext, wield_item, visual_size, nametag_color, nametag_bgcolor,
+		spritediv, initial_sprite_basepos, stepheight, automatic_rotate,
+		automatic_face_movement_dir_offset, automatic_face_movement_max_rotation_per_sec,
+		eye_height, zoom_fov, hp_max, breath_max, glow, pointable, physical,
+		collideWithObjects, rotate_selectionbox, is_visible, makes_footstep_sound,
+		automatic_face_movement_dir, backface_culling, static_save, use_texture_alpha,
+		shaded, show_on_minimap
+		);
+	}
+
+public:
+	bool operator==(const ObjectProperties &other) const {
+		return tie() == other.tie();
+	};
+	bool operator!=(const ObjectProperties &other) const {
+		return tie() != other.tie();
+	};
+
 	/**
 	 * Check limits of some important properties that'd cause exceptions later on.
 	 * Errornous values are discarded after printing a warning.

src/player.h

@@ -27,6 +27,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include <list>
 #include <mutex>
 #include <functional>
+#include <tuple>
 
 #define PLAYERNAME_SIZE 20
 
@@ -42,7 +43,17 @@ struct PlayerFovSpec
 
 	// The time to be take to trasition to the new FOV value.
 	// Transition is instantaneous if omitted. Omitted by default.
-	f32 transition_time;
+	f32 transition_time = 0;
+
+	inline bool operator==(const PlayerFovSpec &other) const {
+		// transition_time is compared here since that could be relevant
+		// when aborting a running transition.
+		return fov == other.fov && is_multiplier == other.is_multiplier &&
+			transition_time == other.transition_time;
+	}
+	inline bool operator!=(const PlayerFovSpec &other) const {
+		return !(*this == other);
+	}
 };
 
 struct PlayerControl
@@ -115,6 +126,24 @@ struct PlayerPhysicsOverride
 	float liquid_sink = 1.f;
 	float acceleration_default = 1.f;
 	float acceleration_air = 1.f;
+
+private:
+	auto tie() const {
+		// Make sure to add new members to this list!
+		return std::tie(
+		speed, jump, gravity, sneak, sneak_glitch, new_move, speed_climb, speed_crouch,
+		liquid_fluidity, liquid_fluidity_smooth, liquid_sink, acceleration_default,
+		acceleration_air
+		);
+	}
+
+public:
+	bool operator==(const PlayerPhysicsOverride &other) const {
+		return tie() == other.tie();
+	};
+	bool operator!=(const PlayerPhysicsOverride &other) const {
+		return tie() != other.tie();
+	};
 };
 
 struct PlayerSettings
@@ -215,9 +244,12 @@ public:
 	void setWieldIndex(u16 index);
 	u16 getWieldIndex() const { return m_wield_index; }
 
-	void setFov(const PlayerFovSpec &spec)
+	bool setFov(const PlayerFovSpec &spec)
 	{
+		if (m_fov_override_spec == spec)
+			return false;
 		m_fov_override_spec = spec;
+		return true;
 	}
 
 	const PlayerFovSpec &getFov() const

src/script/common/c_content.cpp

@@ -412,9 +412,12 @@ void read_object_properties(lua_State *L, int index,
 		prop->automatic_face_movement_dir_offset = luaL_checknumber(L, -1);
 	} else if (lua_isboolean(L, -1)) {
 		prop->automatic_face_movement_dir = lua_toboolean(L, -1);
-		prop->automatic_face_movement_dir_offset = 0.0;
+		prop->automatic_face_movement_dir_offset = 0;
 	}
 	lua_pop(L, 1);
+	getfloatfield(L, -1, "automatic_face_movement_max_rotation_per_sec",
+		prop->automatic_face_movement_max_rotation_per_sec);
+
 	getboolfield(L, -1, "backface_culling", prop->backface_culling);
 	getintfield(L, -1, "glow", prop->glow);
 
@@ -438,12 +441,6 @@ void read_object_properties(lua_State *L, int index,
 	}
 	lua_pop(L, 1);
 
-	lua_getfield(L, -1, "automatic_face_movement_max_rotation_per_sec");
-	if (lua_isnumber(L, -1)) {
-		prop->automatic_face_movement_max_rotation_per_sec = luaL_checknumber(L, -1);
-	}
-	lua_pop(L, 1);
-
 	getstringfield(L, -1, "infotext", prop->infotext);
 	getboolfield(L, -1, "static_save", prop->static_save);
 
@@ -464,7 +461,7 @@ void read_object_properties(lua_State *L, int index,
 }
 
 /******************************************************************************/
-void push_object_properties(lua_State *L, ObjectProperties *prop)
+void push_object_properties(lua_State *L, const ObjectProperties *prop)
 {
 	lua_newtable(L);
 	lua_pushnumber(L, prop->hp_max);
@@ -525,6 +522,8 @@ void push_object_properties(lua_State *L, ObjectProperties *prop)
 	else
 		lua_pushboolean(L, false);
 	lua_setfield(L, -2, "automatic_face_movement_dir");
+	lua_pushnumber(L, prop->automatic_face_movement_max_rotation_per_sec);
+	lua_setfield(L, -2, "automatic_face_movement_max_rotation_per_sec");
 	lua_pushboolean(L, prop->backface_culling);
 	lua_setfield(L, -2, "backface_culling");
 	lua_pushnumber(L, prop->glow);
@@ -540,8 +539,6 @@ void push_object_properties(lua_State *L, ObjectProperties *prop)
 		lua_pushboolean(L, false);
 		lua_setfield(L, -2, "nametag_bgcolor");
 	}
-	lua_pushnumber(L, prop->automatic_face_movement_max_rotation_per_sec);
-	lua_setfield(L, -2, "automatic_face_movement_max_rotation_per_sec");
 	lua_pushlstring(L, prop->infotext.c_str(), prop->infotext.size());
 	lua_setfield(L, -2, "infotext");
 	lua_pushboolean(L, prop->static_save);

src/script/common/c_content.h

@@ -130,7 +130,7 @@ void               read_object_properties    (lua_State *L, int index,
                                               IItemDefManager *idef);
 
 void               push_object_properties    (lua_State *L,
-                                              ObjectProperties *prop);
+                                              const ObjectProperties *prop);
 
 void               push_inventory_list       (lua_State *L,
                                               const InventoryList &invlist);

src/script/lua_api/l_object.cpp

@@ -804,13 +804,16 @@ int ObjectRef::l_set_properties(lua_State *L)
 	if (sao == nullptr)
 		return 0;
 
-	ObjectProperties *prop = sao->accessObjectProperties();
+	auto *prop = sao->accessObjectProperties();
 	if (prop == nullptr)
 		return 0;
 
+	const auto old = *prop;
 	read_object_properties(L, 2, sao, prop, getServer(L)->idef());
-	prop->validate();
-	sao->notifyObjectPropertiesModified();
+	if (*prop != old) {
+		prop->validate();
+		sao->notifyObjectPropertiesModified();
+	}
 	return 0;
 }
 
@@ -1124,7 +1127,7 @@ int ObjectRef::l_get_entity_name(lua_State *L)
 	NO_MAP_LOCK_REQUIRED;
 	ObjectRef *ref = checkObject<ObjectRef>(L, 1);
 	LuaEntitySAO *entitysao = getluaobject(ref);
-	log_deprecated(L,"Deprecated call to \"get_entity_name");
+	log_deprecated(L, "Deprecated call to \"get_entity_name\"");
 	if (entitysao == nullptr)
 		return 0;
 
@@ -1323,13 +1326,14 @@ int ObjectRef::l_set_fov(lua_State *L)
 	if (player == nullptr)
 		return 0;
 
-	float degrees = static_cast<f32>(luaL_checknumber(L, 2));
-	bool is_multiplier = readParam<bool>(L, 3, false);
-	float transition_time = lua_isnumber(L, 4) ?
-		static_cast<f32>(luaL_checknumber(L, 4)) : 0.0f;
+	PlayerFovSpec s;
+	s.fov = readParam<float>(L, 2);
+	s.is_multiplier = readParam<bool>(L, 3, false);
+	if (lua_isnumber(L, 4))
+		s.transition_time = readParam<float>(L, 4);
 
-	player->setFov({degrees, is_multiplier, transition_time});
-	getServer(L)->SendPlayerFov(player->getPeerId());
+	if (player->setFov(s))
+		getServer(L)->SendPlayerFov(player->getPeerId());
 	return 0;
 }
 
@@ -1342,7 +1346,7 @@ int ObjectRef::l_get_fov(lua_State *L)
 	if (player == nullptr)
 		return 0;
 
-	PlayerFovSpec fov_spec = player->getFov();
+	const auto &fov_spec = player->getFov();
 
 	lua_pushnumber(L, fov_spec.fov);
 	lua_pushboolean(L, fov_spec.is_multiplier);
@@ -1446,10 +1450,12 @@ int ObjectRef::l_set_inventory_formspec(lua_State *L)
 	if (player == nullptr)
 		return 0;
 
-	std::string formspec = luaL_checkstring(L, 2);
+	auto formspec = readParam<std::string_view>(L, 2);
 
-	player->inventory_formspec = formspec;
-	getServer(L)->reportInventoryFormspecModified(player->getName());
+	if (formspec != player->inventory_formspec) {
+		player->inventory_formspec = formspec;
+		getServer(L)->reportInventoryFormspecModified(player->getName());
+	}
 	return 0;
 }
 
@@ -1462,7 +1468,7 @@ int ObjectRef::l_get_inventory_formspec(lua_State *L)
 	if (player == nullptr)
 		return 0;
 
-	std::string formspec = player->inventory_formspec;
+	auto &formspec = player->inventory_formspec;
 
 	lua_pushlstring(L, formspec.c_str(), formspec.size());
 	return 1;
@@ -1477,10 +1483,12 @@ int ObjectRef::l_set_formspec_prepend(lua_State *L)
 	if (player == nullptr)
 		return 0;
 
-	std::string formspec = luaL_checkstring(L, 2);
+	auto formspec = readParam<std::string_view>(L, 2);
 
-	player->formspec_prepend = formspec;
-	getServer(L)->reportFormspecPrependModified(player->getName());
+	if (player->formspec_prepend != formspec) {
+		player->formspec_prepend = formspec;
+		getServer(L)->reportFormspecPrependModified(player->getName());
+	}
 	return 0;
 }
 
@@ -1493,7 +1501,7 @@ int ObjectRef::l_get_formspec_prepend(lua_State *L)
 	if (player == nullptr)
 		 return 0;
 
-	std::string formspec = player->formspec_prepend;
+	auto &formspec = player->formspec_prepend;
 
 	lua_pushlstring(L, formspec.c_str(), formspec.size());
 	return 1;
@@ -1506,7 +1514,7 @@ int ObjectRef::l_get_player_control(lua_State *L)
 	ObjectRef *ref = checkObject<ObjectRef>(L, 1);
 	RemotePlayer *player = getplayer(ref);
 
-	lua_newtable(L);
+	lua_createtable(L, 0, 12);
 	if (player == nullptr)
 		return 1;
 
@@ -1580,41 +1588,26 @@ int ObjectRef::l_set_physics_override(lua_State *L)
 	RemotePlayer *player = playersao->getPlayer();
 	auto &phys = player->physics_override;
 
-	if (lua_istable(L, 2)) {
-		bool modified = false;
-		modified |= getfloatfield(L, 2, "speed", phys.speed);
-		modified |= getfloatfield(L, 2, "jump", phys.jump);
-		modified |= getfloatfield(L, 2, "gravity", phys.gravity);
-		modified |= getboolfield(L, 2, "sneak", phys.sneak);
-		modified |= getboolfield(L, 2, "sneak_glitch", phys.sneak_glitch);
-		modified |= getboolfield(L, 2, "new_move", phys.new_move);
-		modified |= getfloatfield(L, 2, "speed_climb", phys.speed_climb);
-		modified |= getfloatfield(L, 2, "speed_crouch", phys.speed_crouch);
-		modified |= getfloatfield(L, 2, "liquid_fluidity", phys.liquid_fluidity);
-		modified |= getfloatfield(L, 2, "liquid_fluidity_smooth", phys.liquid_fluidity_smooth);
-		modified |= getfloatfield(L, 2, "liquid_sink", phys.liquid_sink);
-		modified |= getfloatfield(L, 2, "acceleration_default", phys.acceleration_default);
-		modified |= getfloatfield(L, 2, "acceleration_air", phys.acceleration_air);
-		if (modified)
-			playersao->m_physics_override_sent = false;
-	} else {
-		// old, non-table format
-		// TODO: Remove this code after version 5.4.0
-		log_deprecated(L, "Deprecated use of set_physics_override(num, num, num)");
+	const PlayerPhysicsOverride old = phys;
 
-		if (!lua_isnil(L, 2)) {
-			phys.speed = lua_tonumber(L, 2);
-			playersao->m_physics_override_sent = false;
-		}
-		if (!lua_isnil(L, 3)) {
-			phys.jump = lua_tonumber(L, 3);
-			playersao->m_physics_override_sent = false;
-		}
-		if (!lua_isnil(L, 4)) {
-			phys.gravity = lua_tonumber(L, 4);
-			playersao->m_physics_override_sent = false;
-		}
-	}
+	luaL_checktype(L, 2, LUA_TTABLE);
+
+	getfloatfield(L, 2, "speed", phys.speed);
+	getfloatfield(L, 2, "jump", phys.jump);
+	getfloatfield(L, 2, "gravity", phys.gravity);
+	getboolfield(L, 2, "sneak", phys.sneak);
+	getboolfield(L, 2, "sneak_glitch", phys.sneak_glitch);
+	getboolfield(L, 2, "new_move", phys.new_move);
+	getfloatfield(L, 2, "speed_climb", phys.speed_climb);
+	getfloatfield(L, 2, "speed_crouch", phys.speed_crouch);
+	getfloatfield(L, 2, "liquid_fluidity", phys.liquid_fluidity);
+	getfloatfield(L, 2, "liquid_fluidity_smooth", phys.liquid_fluidity_smooth);
+	getfloatfield(L, 2, "liquid_sink", phys.liquid_sink);
+	getfloatfield(L, 2, "acceleration_default", phys.acceleration_default);
+	getfloatfield(L, 2, "acceleration_air", phys.acceleration_air);
+
+	if (phys != old)
+		playersao->m_physics_override_sent = false;
 	return 0;
 }
 
@@ -1717,6 +1710,7 @@ int ObjectRef::l_hud_change(lua_State *L)
 	void *value = nullptr;
 	bool ok = read_hud_change(L, stat, elem, &value);
 
+	// FIXME: only send when actually changed
 	if (ok)
 		getServer(L)->hudChange(player, id, stat, value);
 
@@ -1836,7 +1830,7 @@ int ObjectRef::l_hud_get_hotbar_itemcount(lua_State *L)
 	if (player == nullptr)
 		return 0;
 
-	lua_pushnumber(L, player->getHotbarItemcount());
+	lua_pushinteger(L, player->getHotbarItemcount());
 	return 1;
 }
 

src/server.cpp

@@ -1789,7 +1789,7 @@ void Server::SendHUDSetFlags(session_t peer_id, u32 flags, u32 mask)
 	Send(&pkt);
 }
 
-void Server::SendHUDSetParam(session_t peer_id, u16 param, const std::string &value)
+void Server::SendHUDSetParam(session_t peer_id, u16 param, std::string_view value)
 {
 	NetworkPacket pkt(TOCLIENT_HUD_SET_PARAM, 0, peer_id);
 	pkt << param << value;
@@ -3433,6 +3433,9 @@ bool Server::hudSetHotbarItemcount(RemotePlayer *player, s32 hotbar_itemcount)
 	if (hotbar_itemcount <= 0 || hotbar_itemcount > HUD_HOTBAR_ITEMCOUNT_MAX)
 		return false;
 
+	if (player->getHotbarItemcount() == hotbar_itemcount)
+		return true;
+
 	player->setHotbarItemcount(hotbar_itemcount);
 	std::ostringstream os(std::ios::binary);
 	writeS32(os, hotbar_itemcount);
@@ -3445,6 +3448,9 @@ void Server::hudSetHotbarImage(RemotePlayer *player, const std::string &name)
 	if (!player)
 		return;
 
+	if (player->getHotbarImage() == name)
+		return;
+
 	player->setHotbarImage(name);
 	SendHUDSetParam(player->getPeerId(), HUD_PARAM_HOTBAR_IMAGE, name);
 }
@@ -3454,6 +3460,9 @@ void Server::hudSetHotbarSelectedImage(RemotePlayer *player, const std::string &
 	if (!player)
 		return;
 
+	if (player->getHotbarSelectedImage() == name)
+		return;
+
 	player->setHotbarSelectedImage(name);
 	SendHUDSetParam(player->getPeerId(), HUD_PARAM_HOTBAR_SELECTED_IMAGE, name);
 }
@@ -3472,9 +3481,13 @@ void Server::setLocalPlayerAnimations(RemotePlayer *player,
 	SendLocalPlayerAnimations(player->getPeerId(), animation_frames, frame_speed);
 }
 
-void Server::setPlayerEyeOffset(RemotePlayer *player, const v3f &first, const v3f &third, const v3f &third_front)
+void Server::setPlayerEyeOffset(RemotePlayer *player, v3f first, v3f third, v3f third_front)
 {
 	sanity_check(player);
+	if (std::tie(player->eye_offset_first, player->eye_offset_third,
+			player->eye_offset_third_front) == std::tie(first, third, third_front))
+		return; // no change
+
 	player->eye_offset_first = first;
 	player->eye_offset_third = third;
 	player->eye_offset_third_front = third_front;

src/server.h

@@ -343,7 +343,7 @@ public:
 
 	void setLocalPlayerAnimations(RemotePlayer *player, v2s32 animation_frames[4],
 			f32 frame_speed);
-	void setPlayerEyeOffset(RemotePlayer *player, const v3f &first, const v3f &third, const v3f &third_front);
+	void setPlayerEyeOffset(RemotePlayer *player, v3f first, v3f third, v3f third_front);
 
 	void setSky(RemotePlayer *player, const SkyboxParams &params);
 	void setSun(RemotePlayer *player, const SunParams &params);
@@ -496,7 +496,7 @@ private:
 	void SendHUDRemove(session_t peer_id, u32 id);
 	void SendHUDChange(session_t peer_id, u32 id, HudElementStat stat, void *value);
 	void SendHUDSetFlags(session_t peer_id, u32 flags, u32 mask);
-	void SendHUDSetParam(session_t peer_id, u16 param, const std::string &value);
+	void SendHUDSetParam(session_t peer_id, u16 param, std::string_view value);
 	void SendSetSky(session_t peer_id, const SkyboxParams &params);
 	void SendSetSun(session_t peer_id, const SunParams &params);
 	void SendSetMoon(session_t peer_id, const MoonParams &params);

src/server/luaentity_sao.cpp

@@ -134,12 +134,16 @@ void LuaEntitySAO::dispatchScriptDeactivate(bool removal)
 
 void LuaEntitySAO::step(float dtime, bool send_recommended)
 {
-	if(!m_properties_sent)
-	{
+	if (!m_properties_sent) {
 		m_properties_sent = true;
 		std::string str = getPropertyPacket();
 		// create message and add to list
-		m_messages_out.emplace(getId(), true, str);
+		m_messages_out.emplace(getId(), true, std::move(str));
+	}
+
+	if (!m_texture_modifier_sent) {
+		m_texture_modifier_sent = true;
+		m_messages_out.emplace(getId(), true, generateSetTextureModCommand());
 	}
 
 	// If attached, check that our parent is still there. If it isn't, detach.
@@ -444,24 +448,24 @@ v3f LuaEntitySAO::getAcceleration()
 
 void LuaEntitySAO::setTextureMod(const std::string &mod)
 {
-	m_current_texture_modifier = mod;
-	// create message and add to list
-	m_messages_out.emplace(getId(), true, generateSetTextureModCommand());
+	if (m_texture_modifier == mod)
+		return;
+	m_texture_modifier = mod;
+	m_texture_modifier_sent = false;
 }
 
 std::string LuaEntitySAO::getTextureMod() const
 {
-	return m_current_texture_modifier;
+	return m_texture_modifier;
 }
 
-
 std::string LuaEntitySAO::generateSetTextureModCommand() const
 {
 	std::ostringstream os(std::ios::binary);
 	// command
 	writeU8(os, AO_CMD_SET_TEXTURE_MOD);
 	// parameters
-	os << serializeString16(m_current_texture_modifier);
+	os << serializeString16(m_texture_modifier);
 	return os.str();
 }
 

src/server/luaentity_sao.h

@@ -103,5 +103,7 @@ private:
 	v3f m_last_sent_rotation;
 	float m_last_sent_position_timer = 0.0f;
 	float m_last_sent_move_precision = 0.0f;
-	std::string m_current_texture_modifier = "";
+
+	std::string m_texture_modifier;
+	bool m_texture_modifier_sent = false;
 };

src/server/unit_sao.cpp

@@ -41,6 +41,8 @@ ServerActiveObject *UnitSAO::getParent() const
 
 void UnitSAO::setArmorGroups(const ItemGroupList &armor_groups)
 {
+	if (m_armor_groups == armor_groups)
+		return;
 	m_armor_groups = armor_groups;
 	m_armor_groups_sent = false;
 }
@@ -53,7 +55,10 @@ const ItemGroupList &UnitSAO::getArmorGroups() const
 void UnitSAO::setAnimation(
 		v2f frame_range, float frame_speed, float frame_blend, bool frame_loop)
 {
-	// store these so they can be updated to clients
+	if (std::tie(m_animation_range, m_animation_speed, m_animation_blend,
+			m_animation_loop) ==
+			std::tie(frame_range, frame_speed, frame_blend, frame_loop))
+		return; // no change
 	m_animation_range = frame_range;
 	m_animation_speed = frame_speed;
 	m_animation_blend = frame_blend;
@@ -72,6 +77,8 @@ void UnitSAO::getAnimation(v2f *frame_range, float *frame_speed, float *frame_bl
 
 void UnitSAO::setAnimationSpeed(float frame_speed)
 {
+	if (m_animation_speed == frame_speed)
+		return;
 	m_animation_speed = frame_speed;
 	m_animation_speed_sent = false;
 }