Re-order sound-related code (#12382)

Dropped ServerSoundParams -> moved to ServerPlayingSound. This gets rid of the duplicated
'fade' and 'pitch' values on server-side where only one was used anyway.
SimpleSoundSpec is the basic sound without positional information, hence 'loop' is included.

Recursively added PROTOCOL_VERSION to most functions to reduce the versioning mess in the
future. Per-type version numbers are kept for now as a safety rope in a special case.

src/client/content_cao.cpp

@@ -1174,7 +1174,7 @@ void GenericCAO::step(float dtime, ClientEnvironment *env)
 				// Reduce footstep gain, as non-local-player footsteps are
 				// somehow louder.
 				spec.gain *= 0.6f;
-				m_client->sound()->playSoundAt(spec, false, getPosition());
+				m_client->sound()->playSoundAt(spec, getPosition());
 			}
 		}
 	}

src/client/game.cpp

@@ -282,7 +282,7 @@ public:
 		if (m_player_step_timer <= 0 && m_player_step_sound.exists()) {
 			m_player_step_timer = 0.03;
 			if (makes_footstep_sound)
-				m_sound->playSound(m_player_step_sound, false);
+				m_sound->playSound(m_player_step_sound);
 		}
 	}
 
@@ -290,7 +290,7 @@ public:
 	{
 		if (m_player_jump_timer <= 0.0f) {
 			m_player_jump_timer = 0.2f;
-			m_sound->playSound(SimpleSoundSpec("player_jump", 0.5f), false);
+			m_sound->playSound(SimpleSoundSpec("player_jump", 0.5f));
 		}
 	}
 
@@ -315,32 +315,32 @@ public:
 	static void cameraPunchLeft(MtEvent *e, void *data)
 	{
 		SoundMaker *sm = (SoundMaker *)data;
-		sm->m_sound->playSound(sm->m_player_leftpunch_sound, false);
+		sm->m_sound->playSound(sm->m_player_leftpunch_sound);
 	}
 
 	static void cameraPunchRight(MtEvent *e, void *data)
 	{
 		SoundMaker *sm = (SoundMaker *)data;
-		sm->m_sound->playSound(sm->m_player_rightpunch_sound, false);
+		sm->m_sound->playSound(sm->m_player_rightpunch_sound);
 	}
 
 	static void nodeDug(MtEvent *e, void *data)
 	{
 		SoundMaker *sm = (SoundMaker *)data;
 		NodeDugEvent *nde = (NodeDugEvent *)e;
-		sm->m_sound->playSound(sm->m_ndef->get(nde->n).sound_dug, false);
+		sm->m_sound->playSound(sm->m_ndef->get(nde->n).sound_dug);
 	}
 
 	static void playerDamage(MtEvent *e, void *data)
 	{
 		SoundMaker *sm = (SoundMaker *)data;
-		sm->m_sound->playSound(SimpleSoundSpec("player_damage", 0.5), false);
+		sm->m_sound->playSound(SimpleSoundSpec("player_damage", 0.5));
 	}
 
 	static void playerFallingDamage(MtEvent *e, void *data)
 	{
 		SoundMaker *sm = (SoundMaker *)data;
-		sm->m_sound->playSound(SimpleSoundSpec("player_falling_damage", 0.5), false);
+		sm->m_sound->playSound(SimpleSoundSpec("player_falling_damage", 0.5));
 	}
 
 	void registerReceiver(MtEventManager *mgr)

src/client/sound.h

@@ -63,13 +63,13 @@ public:
 	virtual void step(float dtime) = 0;
 	virtual void fadeSound(int sound, float step, float gain) = 0;
 
-	int playSound(const SimpleSoundSpec &spec, bool loop)
+	int playSound(const SimpleSoundSpec &spec)
 	{
-		return playSound(spec.name, loop, spec.gain, spec.fade, spec.pitch);
+		return playSound(spec.name, spec.loop, spec.gain, spec.fade, spec.pitch);
 	}
-	int playSoundAt(const SimpleSoundSpec &spec, bool loop, const v3f &pos)
+	int playSoundAt(const SimpleSoundSpec &spec, const v3f &pos)
 	{
-		return playSoundAt(spec.name, loop, spec.gain, pos, spec.pitch);
+		return playSoundAt(spec.name, spec.loop, spec.gain, pos, spec.pitch);
 	}
 };
 

src/gui/guiEngine.cpp

@@ -622,9 +622,9 @@ void GUIEngine::updateTopLeftTextSize()
 }
 
 /******************************************************************************/
-s32 GUIEngine::playSound(const SimpleSoundSpec &spec, bool looped)
+s32 GUIEngine::playSound(const SimpleSoundSpec &spec)
 {
-	s32 handle = m_sound_manager->playSound(spec, looped);
+	s32 handle = m_sound_manager->playSound(spec);
 	return handle;
 }
 

src/gui/guiEngine.h

@@ -296,7 +296,7 @@ private:
 	clouddata   m_cloud;
 
 	/** start playing a sound and return handle */
-	s32 playSound(const SimpleSoundSpec &spec, bool looped);
+	s32 playSound(const SimpleSoundSpec &spec);
 	/** stop playing a sound started with playSound() */
 	void stopSound(s32 handle);
 

src/itemdef.cpp

@@ -154,8 +154,8 @@ void ItemDefinition::serialize(std::ostream &os, u16 protocol_version) const
 	os << serializeString16(node_placement_prediction);
 
 	// Version from ContentFeatures::serialize to keep in sync
-	sound_place.serialize(os, CONTENTFEATURES_VERSION);
+	sound_place.serialize(os, protocol_version);
-	sound_place_failed.serialize(os, CONTENTFEATURES_VERSION);
+	sound_place_failed.serialize(os, protocol_version);
 
 	writeF32(os, range);
 	os << serializeString16(palette_image);
@@ -168,7 +168,7 @@ void ItemDefinition::serialize(std::ostream &os, u16 protocol_version) const
 	os << place_param2;
 }
 
-void ItemDefinition::deSerialize(std::istream &is)
+void ItemDefinition::deSerialize(std::istream &is, u16 protocol_version)
 {
 	// Reset everything
 	reset();
@@ -205,9 +205,8 @@ void ItemDefinition::deSerialize(std::istream &is)
 
 	node_placement_prediction = deSerializeString16(is);
 
-	// Version from ContentFeatures::serialize to keep in sync
+	sound_place.deSerialize(is, protocol_version);
-	sound_place.deSerialize(is, CONTENTFEATURES_VERSION);
+	sound_place_failed.deSerialize(is, protocol_version);
-	sound_place_failed.deSerialize(is, CONTENTFEATURES_VERSION);
 
 	range = readF32(is);
 	palette_image = deSerializeString16(is);
@@ -538,21 +537,21 @@ public:
 			os << serializeString16(it.second);
 		}
 	}
-	void deSerialize(std::istream &is)
+	void deSerialize(std::istream &is, u16 protocol_version)
 	{
 		// Clear everything
 		clear();
-		// Deserialize
+
-		int version = readU8(is);
+		if(readU8(is) != 0)
-		if(version != 0)
 			throw SerializationError("unsupported ItemDefManager version");
+
 		u16 count = readU16(is);
 		for(u16 i=0; i<count; i++)
 		{
 			// Deserialize a string and grab an ItemDefinition from it
 			std::istringstream tmp_is(deSerializeString16(is), std::ios::binary);
 			ItemDefinition def;
-			def.deSerialize(tmp_is);
+			def.deSerialize(tmp_is, protocol_version);
 			// Register
 			registerItem(def);
 		}

src/itemdef.h

@@ -97,7 +97,7 @@ struct ItemDefinition
 	~ItemDefinition();
 	void reset();
 	void serialize(std::ostream &os, u16 protocol_version) const;
-	void deSerialize(std::istream &is);
+	void deSerialize(std::istream &is, u16 protocol_version);
 private:
 	void resetInitial();
 };
@@ -177,7 +177,7 @@ public:
 			const std::string &convert_to)=0;
 
 	virtual void serialize(std::ostream &os, u16 protocol_version)=0;
-	virtual void deSerialize(std::istream &is)=0;
+	virtual void deSerialize(std::istream &is, u16 protocol_version)=0;
 
 	// Do stuff asked by threads that can only be done in the main thread
 	virtual void processQueue(IGameDef *gamedef)=0;

src/network/clientpackethandler.cpp

@@ -778,7 +778,7 @@ void Client::handleCommand_NodeDef(NetworkPacket* pkt)
 	decompressZlib(tmp_is, tmp_os);
 
 	// Deserialize node definitions
-	m_nodedef->deSerialize(tmp_os);
+	m_nodedef->deSerialize(tmp_os, m_proto_ver);
 	m_nodedef_received = true;
 }
 
@@ -797,7 +797,7 @@ void Client::handleCommand_ItemDef(NetworkPacket* pkt)
 	decompressZlib(tmp_is, tmp_os);
 
 	// Deserialize node definitions
-	m_itemdef->deSerialize(tmp_os);
+	m_itemdef->deSerialize(tmp_os, m_proto_ver);
 	m_itemdef_received = true;
 }
 
@@ -818,22 +818,18 @@ void Client::handleCommand_PlaySound(NetworkPacket* pkt)
 	*/
 
 	s32 server_id;
-	std::string name;
 
-	float gain;
+	SimpleSoundSpec spec;
 	u8 type; // 0=local, 1=positional, 2=object
 	v3f pos;
 	u16 object_id;
-	bool loop;
-	float fade = 0.0f;
-	float pitch = 1.0f;
 	bool ephemeral = false;
 
-	*pkt >> server_id >> name >> gain >> type >> pos >> object_id >> loop;
+	*pkt >> server_id >> spec.name >> spec.gain >> type >> pos >> object_id >> spec.loop;
 
 	try {
-		*pkt >> fade;
+		*pkt >> spec.fade;
-		*pkt >> pitch;
+		*pkt >> spec.pitch;
 		*pkt >> ephemeral;
 	} catch (PacketError &e) {};
 
@@ -841,17 +837,17 @@ void Client::handleCommand_PlaySound(NetworkPacket* pkt)
 	int client_id = -1;
 	switch(type) {
 		case 0: // local
-			client_id = m_sound->playSound(name, loop, gain, fade, pitch);
+			client_id = m_sound->playSound(spec);
 			break;
 		case 1: // positional
-			client_id = m_sound->playSoundAt(name, loop, gain, pos, pitch);
+			client_id = m_sound->playSoundAt(spec, pos);
 			break;
 		case 2:
 		{ // object
 			ClientActiveObject *cao = m_env.getActiveObject(object_id);
 			if (cao)
 				pos = cao->getPosition();
-			client_id = m_sound->playSoundAt(name, loop, gain, pos, pitch);
+			client_id = m_sound->playSoundAt(spec, pos);
 			break;
 		}
 		default:

src/nodedef.cpp

@@ -61,9 +61,7 @@ void NodeBox::reset()
 
 void NodeBox::serialize(std::ostream &os, u16 protocol_version) const
 {
-	// Protocol >= 36
+	writeU8(os, 6); // version. Protocol >= 36
-	const u8 version = 6;
-	writeU8(os, version);
 
 	switch (type) {
 	case NODEBOX_LEVELED:
@@ -123,8 +121,7 @@ void NodeBox::serialize(std::ostream &os, u16 protocol_version) const
 
 void NodeBox::deSerialize(std::istream &is)
 {
-	int version = readU8(is);
+	if (readU8(is) < 6)
-	if (version < 6)
 		throw SerializationError("unsupported NodeBox version");
 
 	reset();
@@ -249,14 +246,13 @@ void TileDef::serialize(std::ostream &os, u16 protocol_version) const
 		writeU8(os, align_style);
 }
 
-void TileDef::deSerialize(std::istream &is, u8 contentfeatures_version,
+void TileDef::deSerialize(std::istream &is, NodeDrawType drawtype, u16 protocol_version)
-	NodeDrawType drawtype)
 {
-	int version = readU8(is);
+	if (readU8(is) < 6)
-	if (version < 6)
 		throw SerializationError("unsupported TileDef version");
+
 	name = deSerializeString16(is);
-	animation.deSerialize(is, version);
+	animation.deSerialize(is, protocol_version);
 	u16 flags = readU16(is);
 	backface_culling = flags & TILE_FLAG_BACKFACE_CULLING;
 	tileable_horizontal = flags & TILE_FLAG_TILEABLE_HORIZONTAL;
@@ -448,8 +444,7 @@ u8 ContentFeatures::getAlphaForLegacy() const
 
 void ContentFeatures::serialize(std::ostream &os, u16 protocol_version) const
 {
-	const u8 version = CONTENTFEATURES_VERSION;
+	writeU8(os, CONTENTFEATURES_VERSION);
-	writeU8(os, version);
 
 	// general
 	os << serializeString16(name);
@@ -534,9 +529,9 @@ void ContentFeatures::serialize(std::ostream &os, u16 protocol_version) const
 	collision_box.serialize(os, protocol_version);
 
 	// sound
-	sound_footstep.serialize(os, version);
+	sound_footstep.serialize(os, protocol_version);
-	sound_dig.serialize(os, version);
+	sound_dig.serialize(os, protocol_version);
-	sound_dug.serialize(os, version);
+	sound_dug.serialize(os, protocol_version);
 
 	// legacy
 	writeU8(os, legacy_facedir_simple);
@@ -550,11 +545,9 @@ void ContentFeatures::serialize(std::ostream &os, u16 protocol_version) const
 	writeU8(os, liquid_move_physics);
 }
 
-void ContentFeatures::deSerialize(std::istream &is)
+void ContentFeatures::deSerialize(std::istream &is, u16 protocol_version)
 {
-	// version detection
+	if (readU8(is) < CONTENTFEATURES_VERSION)
-	const u8 version = readU8(is);
-	if (version < CONTENTFEATURES_VERSION)
 		throw SerializationError("unsupported ContentFeatures version");
 
 	// general
@@ -576,13 +569,13 @@ void ContentFeatures::deSerialize(std::istream &is)
 	if (readU8(is) != 6)
 		throw SerializationError("unsupported tile count");
 	for (TileDef &td : tiledef)
-		td.deSerialize(is, version, drawtype);
+		td.deSerialize(is, drawtype, protocol_version);
 	for (TileDef &td : tiledef_overlay)
-		td.deSerialize(is, version, drawtype);
+		td.deSerialize(is, drawtype, protocol_version);
 	if (readU8(is) != CF_SPECIAL_COUNT)
 		throw SerializationError("unsupported CF_SPECIAL_COUNT");
 	for (TileDef &td : tiledef_special)
-		td.deSerialize(is, version, drawtype);
+		td.deSerialize(is, drawtype, protocol_version);
 	setAlphaFromLegacy(readU8(is));
 	color.setRed(readU8(is));
 	color.setGreen(readU8(is));
@@ -633,9 +626,9 @@ void ContentFeatures::deSerialize(std::istream &is)
 	collision_box.deSerialize(is);
 
 	// sounds
-	sound_footstep.deSerialize(is, version);
+	sound_footstep.deSerialize(is, protocol_version);
-	sound_dig.deSerialize(is, version);
+	sound_dig.deSerialize(is, protocol_version);
-	sound_dug.deSerialize(is, version);
+	sound_dug.deSerialize(is, protocol_version);
 
 	// read legacy properties
 	legacy_facedir_simple = readU8(is);
@@ -1546,12 +1539,13 @@ void NodeDefManager::serialize(std::ostream &os, u16 protocol_version) const
 }
 
 
-void NodeDefManager::deSerialize(std::istream &is)
+void NodeDefManager::deSerialize(std::istream &is, u16 protocol_version)
 {
 	clear();
-	int version = readU8(is);
+
-	if (version != 1)
+	if (readU8(is) < 1)
 		throw SerializationError("unsupported NodeDefinitionManager version");
+
 	u16 count = readU16(is);
 	std::istringstream is2(deSerializeString32(is), std::ios::binary);
 	ContentFeatures f;
@@ -1561,7 +1555,7 @@ void NodeDefManager::deSerialize(std::istream &is)
 		// Read it from the string wrapper
 		std::string wrapper = deSerializeString16(is2);
 		std::istringstream wrapper_is(wrapper, std::ios::binary);
-		f.deSerialize(wrapper_is);
+		f.deSerialize(wrapper_is, protocol_version);
 
 		// Check error conditions
 		if (i == CONTENT_IGNORE || i == CONTENT_AIR || i == CONTENT_UNKNOWN) {

src/nodedef.h

@@ -36,9 +36,6 @@ class Client;
 #include "texture_override.h" // TextureOverride
 #include "tileanimation.h"
 
-// PROTOCOL_VERSION >= 37
-static const u8 CONTENTFEATURES_VERSION = 13;
-
 class IItemDefManager;
 class ITextureSource;
 class IShaderSource;
@@ -286,8 +283,7 @@ struct TileDef
 	}
 
 	void serialize(std::ostream &os, u16 protocol_version) const;
-	void deSerialize(std::istream &is, u8 contentfeatures_version,
+	void deSerialize(std::istream &is, NodeDrawType drawtype, u16 protocol_version);
-		NodeDrawType drawtype);
 };
 
 // Defines the number of special tiles per nodedef
@@ -299,6 +295,10 @@ struct TileDef
 
 struct ContentFeatures
 {
+	// PROTOCOL_VERSION >= 37. This is legacy and should not be increased anymore, 
+	// write checks that depend directly on the protocol version instead.
+	static const u8 CONTENTFEATURES_VERSION = 13;
+
 	/*
 		Cached stuff
 	 */
@@ -447,7 +447,7 @@ struct ContentFeatures
 	~ContentFeatures();
 	void reset();
 	void serialize(std::ostream &os, u16 protocol_version) const;
-	void deSerialize(std::istream &is);
+	void deSerialize(std::istream &is, u16 protocol_version);
 
 	/*
 		Some handy methods
@@ -690,7 +690,7 @@ public:
 
 	/*!
 	 * Writes the content of this manager to the given output stream.
-	 * @param protocol_version serialization version of ContentFeatures
+	 * @param protocol_version Active network protocol version
 	 */
 	void serialize(std::ostream &os, u16 protocol_version) const;
 
@@ -698,8 +698,9 @@ public:
 	 * Restores the manager from a serialized stream.
 	 * This clears the previous state.
 	 * @param is input stream containing a serialized NodeDefManager
+	 * @param protocol_version Active network protocol version
 	 */
-	void deSerialize(std::istream &is);
+	void deSerialize(std::istream &is, u16 protocol_version);
 
 	/*!
 	 * Used to indicate that node registration has finished.

src/script/common/c_content.cpp

@@ -1051,22 +1051,26 @@ void push_palette(lua_State *L, const std::vector<video::SColor> *palette)
 
 /******************************************************************************/
 void read_server_sound_params(lua_State *L, int index,
-		ServerSoundParams &params)
+		ServerPlayingSound &params)
 {
 	if(index < 0)
 		index = lua_gettop(L) + 1 + index;
-	// Clear
+
-	params = ServerSoundParams();
 	if(lua_istable(L, index)){
+		// Functional overlap: this may modify SimpleSoundSpec contents
+		getfloatfield(L, index, "fade", params.spec.fade);
+		getfloatfield(L, index, "pitch", params.spec.pitch);
+		getboolfield(L, index, "loop", params.spec.loop);
+
 		getfloatfield(L, index, "gain", params.gain);
+
+		// Handle positional information
 		getstringfield(L, index, "to_player", params.to_player);
-		getfloatfield(L, index, "fade", params.fade);
-		getfloatfield(L, index, "pitch", params.pitch);
 		lua_getfield(L, index, "pos");
 		if(!lua_isnil(L, -1)){
 			v3f p = read_v3f(L, -1)*BS;
 			params.pos = p;
-			params.type = ServerSoundParams::SSP_POSITIONAL;
+			params.type = ServerPlayingSound::SSP_POSITIONAL;
 		}
 		lua_pop(L, 1);
 		lua_getfield(L, index, "object");
@@ -1075,13 +1079,12 @@ void read_server_sound_params(lua_State *L, int index,
 			ServerActiveObject *sao = ObjectRef::getobject(ref);
 			if(sao){
 				params.object = sao->getId();
-				params.type = ServerSoundParams::SSP_OBJECT;
+				params.type = ServerPlayingSound::SSP_OBJECT;
 			}
 		}
 		lua_pop(L, 1);
 		params.max_hear_distance = BS*getfloatfield_default(L, index,
 				"max_hear_distance", params.max_hear_distance/BS);
-		getboolfield(L, index, "loop", params.loop);
 		getstringfield(L, index, "exclude_player", params.exclude_player);
 	}
 }

src/script/common/c_content.h

@@ -53,7 +53,7 @@ struct ItemDefinition;
 struct ToolCapabilities;
 struct ObjectProperties;
 struct SimpleSoundSpec;
-struct ServerSoundParams;
+struct ServerPlayingSound;
 class Inventory;
 class InventoryList;
 struct NodeBox;
@@ -91,7 +91,7 @@ void               read_soundspec            (lua_State *L, int index,
 NodeBox            read_nodebox              (lua_State *L, int index);
 
 void               read_server_sound_params  (lua_State *L, int index,
-                                              ServerSoundParams &params);
+                                              ServerPlayingSound &params);
 
 void               push_dig_params           (lua_State *L,
                                               const DigParams &params);

src/script/lua_api/l_server.cpp

@@ -437,16 +437,15 @@ int ModApiServer::l_get_worldpath(lua_State *L)
 int ModApiServer::l_sound_play(lua_State *L)
 {
 	NO_MAP_LOCK_REQUIRED;
-	SimpleSoundSpec spec;
+	ServerPlayingSound params;
-	read_soundspec(L, 1, spec);
+	read_soundspec(L, 1, params.spec);
-	ServerSoundParams params;
 	read_server_sound_params(L, 2, params);
 	bool ephemeral = lua_gettop(L) > 2 && readParam<bool>(L, 3);
 	if (ephemeral) {
-		getServer(L)->playSound(spec, params, true);
+		getServer(L)->playSound(params, true);
 		lua_pushnil(L);
 	} else {
-		s32 handle = getServer(L)->playSound(spec, params);
+		s32 handle = getServer(L)->playSound(params);
 		lua_pushinteger(L, handle);
 	}
 	return 1;

src/script/lua_api/l_sound.cpp

@@ -28,9 +28,9 @@ int ModApiSound::l_sound_play(lua_State *L)
 {
 	SimpleSoundSpec spec;
 	read_soundspec(L, 1, spec);
-	bool looped = readParam<bool>(L, 2);
+	spec.loop = readParam<bool>(L, 2);
 
-	s32 handle = getGuiEngine(L)->playSound(spec, looped);
+	s32 handle = getGuiEngine(L)->playSound(spec);
 
 	lua_pushinteger(L, handle);
 

src/server.cpp

@@ -136,7 +136,7 @@ void *ServerThread::run()
 	return nullptr;
 }
 
-v3f ServerSoundParams::getPos(ServerEnvironment *env, bool *pos_exists) const
+v3f ServerPlayingSound::getPos(ServerEnvironment *env, bool *pos_exists) const
 {
 	if(pos_exists) *pos_exists = false;
 	switch(type){
@@ -2066,14 +2066,13 @@ inline s32 Server::nextSoundId()
 	return ret;
 }
 
-s32 Server::playSound(const SimpleSoundSpec &spec,
+s32 Server::playSound(ServerPlayingSound &params, bool ephemeral)
-		const ServerSoundParams &params, bool ephemeral)
 {
 	// Find out initial position of sound
 	bool pos_exists = false;
 	v3f pos = params.getPos(m_env, &pos_exists);
 	// If position is not found while it should be, cancel sound
-	if(pos_exists != (params.type != ServerSoundParams::SSP_LOCAL))
+	if(pos_exists != (params.type != ServerPlayingSound::SSP_LOCAL))
 		return -1;
 
 	// Filter destination clients
@@ -2118,101 +2117,68 @@ s32 Server::playSound(const SimpleSoundSpec &spec,
 	if(dst_clients.empty())
 		return -1;
 
-	// Create the sound
+	// old clients will still use this, so pick a reserved ID (-1)
-	s32 id;
+	const s32 id = ephemeral ? -1 : nextSoundId();
-	ServerPlayingSound *psound = nullptr;
-	if (ephemeral) {
-		id = -1; // old clients will still use this, so pick a reserved ID
-	} else {
-		id = nextSoundId();
-		// The sound will exist as a reference in m_playing_sounds
-		m_playing_sounds[id] = ServerPlayingSound();
-		psound = &m_playing_sounds[id];
-		psound->params = params;
-		psound->spec = spec;
-	}
 
-	float gain = params.gain * spec.gain;
+	float gain = params.gain * params.spec.gain;
 	NetworkPacket pkt(TOCLIENT_PLAY_SOUND, 0);
-	pkt << id << spec.name << gain
+	pkt << id << params.spec.name << gain
 			<< (u8) params.type << pos << params.object
-			<< params.loop << params.fade << params.pitch
+			<< params.spec.loop << params.spec.fade << params.spec.pitch
 			<< ephemeral;
 
 	bool as_reliable = !ephemeral;
 
-	for (const u16 dst_client : dst_clients) {
+	for (const session_t peer_id : dst_clients) {
-		if (psound)
+		if (!ephemeral)
-			psound->clients.insert(dst_client);
+			params.clients.insert(peer_id);
-		m_clients.send(dst_client, 0, &pkt, as_reliable);
+		m_clients.send(peer_id, 0, &pkt, as_reliable);
 	}
+
+	if (!ephemeral)
+		m_playing_sounds[id] = std::move(params);
 	return id;
 }
 void Server::stopSound(s32 handle)
 {
-	// Get sound reference
+	auto it = m_playing_sounds.find(handle);
-	std::unordered_map<s32, ServerPlayingSound>::iterator i =
+	if (it == m_playing_sounds.end())
-		m_playing_sounds.find(handle);
-	if (i == m_playing_sounds.end())
 		return;
-	ServerPlayingSound &psound = i->second;
+
+	ServerPlayingSound &psound = it->second;
 
 	NetworkPacket pkt(TOCLIENT_STOP_SOUND, 4);
 	pkt << handle;
 
-	for (std::unordered_set<session_t>::const_iterator si = psound.clients.begin();
+	for (session_t peer_id : psound.clients) {
-			si != psound.clients.end(); ++si) {
 		// Send as reliable
-		m_clients.send(*si, 0, &pkt, true);
+		m_clients.send(peer_id, 0, &pkt, true);
 	}
+
 	// Remove sound reference
-	m_playing_sounds.erase(i);
+	m_playing_sounds.erase(it);
 }
 
 void Server::fadeSound(s32 handle, float step, float gain)
 {
-	// Get sound reference
+	auto it = m_playing_sounds.find(handle);
-	std::unordered_map<s32, ServerPlayingSound>::iterator i =
+	if (it == m_playing_sounds.end())
-		m_playing_sounds.find(handle);
-	if (i == m_playing_sounds.end())
 		return;
 
-	ServerPlayingSound &psound = i->second;
+	ServerPlayingSound &psound = it->second;
-	psound.params.gain = gain;
+	psound.gain = gain; // destination gain
 
 	NetworkPacket pkt(TOCLIENT_FADE_SOUND, 4);
 	pkt << handle << step << gain;
 
-	// Backwards compability
+	for (session_t peer_id : psound.clients) {
-	bool play_sound = gain > 0;
-	ServerPlayingSound compat_psound = psound;
-	compat_psound.clients.clear();
-
-	NetworkPacket compat_pkt(TOCLIENT_STOP_SOUND, 4);
-	compat_pkt << handle;
-
-	for (std::unordered_set<u16>::iterator it = psound.clients.begin();
-			it != psound.clients.end();) {
-		if (m_clients.getProtocolVersion(*it) >= 32) {
 		// Send as reliable
-			m_clients.send(*it, 0, &pkt, true);
+		m_clients.send(peer_id, 0, &pkt, true);
-			++it;
-		} else {
-			compat_psound.clients.insert(*it);
-			// Stop old sound
-			m_clients.send(*it, 0, &compat_pkt, true);
-			psound.clients.erase(it++);
-		}
 	}
 
 	// Remove sound reference
-	if (!play_sound || psound.clients.empty())
+	if (gain <= 0 || psound.clients.empty())
-		m_playing_sounds.erase(i);
+		m_playing_sounds.erase(it);
-
-	if (play_sound && !compat_psound.clients.empty()) {
-		// Play new sound volume on older clients
-		playSound(compat_psound.spec, compat_psound.params);
-	}
 }
 
 void Server::sendRemoveNode(v3s16 p, std::unordered_set<u16> *far_players,

src/server.h

@@ -96,30 +96,26 @@ struct MediaInfo
 	}
 };
 
-struct ServerSoundParams
+// Combines the pure sound (SimpleSoundSpec) with positional information
+struct ServerPlayingSound
 {
 	enum Type {
 		SSP_LOCAL,
 		SSP_POSITIONAL,
 		SSP_OBJECT
 	} type = SSP_LOCAL;
-	float gain = 1.0f;
+
-	float fade = 0.0f;
+	float gain = 1.0f; // for amplification of the base sound
-	float pitch = 1.0f;
-	bool loop = false;
 	float max_hear_distance = 32 * BS;
 	v3f pos;
 	u16 object = 0;
-	std::string to_player = "";
+	std::string to_player;
-	std::string exclude_player = "";
+	std::string exclude_player;
 
 	v3f getPos(ServerEnvironment *env, bool *pos_exists) const;
-};
 
-struct ServerPlayingSound
-{
-	ServerSoundParams params;
 	SimpleSoundSpec spec;
+
 	std::unordered_set<session_t> clients; // peer ids
 };
 
@@ -236,8 +232,7 @@ public:
 
 	// Returns -1 if failed, sound handle on success
 	// Envlock
-	s32 playSound(const SimpleSoundSpec &spec, const ServerSoundParams &params,
+	s32 playSound(ServerPlayingSound &params, bool ephemeral=false);
-			bool ephemeral=false);
 	void stopSound(s32 handle);
 	void fadeSound(s32 handle, float step, float gain);
 

src/sound.h

@@ -24,30 +24,28 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "util/serialize.h"
 #include "irrlichttypes_bloated.h"
 
+// This class describes the basic sound information for playback.
+// Positional handling is done separately.
+
 struct SimpleSoundSpec
 {
 	SimpleSoundSpec(const std::string &name = "", float gain = 1.0f,
-			float fade = 0.0f, float pitch = 1.0f) :
+			bool loop = false, float fade = 0.0f, float pitch = 1.0f) :
-			name(name),
+			name(name), gain(gain), fade(fade), pitch(pitch), loop(loop)
-			gain(gain), fade(fade), pitch(pitch)
 	{
 	}
 
 	bool exists() const { return !name.empty(); }
 
-	// Take cf_version from ContentFeatures::serialize to
+	void serialize(std::ostream &os, u16 protocol_version) const
-	// keep in sync with item definitions
-	void serialize(std::ostream &os, u8 cf_version) const
 	{
 		os << serializeString16(name);
 		writeF32(os, gain);
 		writeF32(os, pitch);
 		writeF32(os, fade);
-		// if (cf_version < ?)
-		//     return;
 	}
 
-	void deSerialize(std::istream &is, u8 cf_version)
+	void deSerialize(std::istream &is, u16 protocol_version)
 	{
 		name = deSerializeString16(is);
 		gain = readF32(is);
@@ -59,4 +57,5 @@ struct SimpleSoundSpec
 	float gain = 1.0f;
 	float fade = 0.0f;
 	float pitch = 1.0f;
+	bool loop = false;
 };

src/tileanimation.cpp

@@ -19,7 +19,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "tileanimation.h"
 #include "util/serialize.h"
 
-void TileAnimationParams::serialize(std::ostream &os, u8 tiledef_version) const
+void TileAnimationParams::serialize(std::ostream &os, u16 protocol_version) const
 {
 	writeU8(os, type);
 	if (type == TAT_VERTICAL_FRAMES) {
@@ -33,7 +33,7 @@ void TileAnimationParams::serialize(std::ostream &os, u8 tiledef_version) const
 	}
 }
 
-void TileAnimationParams::deSerialize(std::istream &is, u8 tiledef_version)
+void TileAnimationParams::deSerialize(std::istream &is, u16 protocol_version)
 {
 	type = (TileAnimationType) readU8(is);
 

src/tileanimation.h

@@ -50,8 +50,8 @@ struct TileAnimationParams
 		} sheet_2d;
 	};
 
-	void serialize(std::ostream &os, u8 tiledef_version) const;
+	void serialize(std::ostream &os, u16 protocol_version) const;
-	void deSerialize(std::istream &is, u8 tiledef_version);
+	void deSerialize(std::istream &is, u16 protocol_version);
 	void determineParams(v2u32 texture_size, int *frame_count, int *frame_length_ms,
 			v2u32 *frame_size) const;
 	void getTextureModifer(std::ostream &os, v2u32 texture_size, int frame) const;

src/unittest/test_nodedef.cpp

@@ -60,7 +60,7 @@ void TestNodeDef::testContentFeaturesSerialization()
 
 	std::istringstream is(os.str(), std::ios::binary);
 	ContentFeatures f2;
-	f2.deSerialize(is);
+	f2.deSerialize(is, LATEST_PROTOCOL_VERSION);
 
 	UASSERT(f.walkable == f2.walkable);
 	UASSERT(f.node_box.type == f2.node_box.type);