Sounds: Various little improvements (#12486)

Use SimpleSoundSpec where reasonable (OpenAL)
Ensure the sound IDs do not underflow or get overwritten -> loop in u16
Proper use of an enum.

diff --git a/src/client/sound.h b/src/client/sound.h
index a3bb3ce8728c3cf3faa8d6e630b398d61423f9fd..213d208316e6aed1940b01ccb4017420aff379db 100644 (file)
--- a/src/client/sound.h
+++ b/src/client/sound.h
@@ -51,10 +51,8 @@ class ISoundManager
 
        // playSound functions return -1 on failure, otherwise a handle to the
        // sound. If name=="", call should be ignored without error.
-       virtual int playSound(const std::string &name, bool loop, float volume,
-                       float fade = 0.0f, float pitch = 1.0f) = 0;
-       virtual int playSoundAt(const std::string &name, bool loop, float volume, v3f pos,
-                       float pitch = 1.0f) = 0;
+       virtual int playSound(const SimpleSoundSpec &spec) = 0;
+       virtual int playSoundAt(const SimpleSoundSpec &spec, const v3f &pos) = 0;
        virtual void stopSound(int sound) = 0;
        virtual bool soundExists(int sound) = 0;
        virtual void updateSoundPosition(int sound, v3f pos) = 0;
@@ -62,15 +60,6 @@ class ISoundManager
        virtual float getSoundGain(int id) = 0;
        virtual void step(float dtime) = 0;
        virtual void fadeSound(int sound, float step, float gain) = 0;
-
-       int playSound(const SimpleSoundSpec &spec)
-       {
-               return playSound(spec.name, spec.loop, spec.gain, spec.fade, spec.pitch);
-       }
-       int playSoundAt(const SimpleSoundSpec &spec, const v3f &pos)
-       {
-               return playSoundAt(spec.name, spec.loop, spec.gain, pos, spec.pitch);
-       }
 };
 
 class DummySoundManager : public ISoundManager
@@ -88,16 +77,9 @@ class DummySoundManager : public ISoundManager
        {
        }
        void setListenerGain(float gain) {}
-       int playSound(const std::string &name, bool loop, float volume, float fade,
-                       float pitch)
-       {
-               return 0;
-       }
-       int playSoundAt(const std::string &name, bool loop, float volume, v3f pos,
-                       float pitch)
-       {
-               return 0;
-       }
+
+       int playSound(const SimpleSoundSpec &spec) { return -1; }
+       int playSoundAt(const SimpleSoundSpec &spec, const v3f &pos) { return -1; }
        void stopSound(int sound) {}
        bool soundExists(int sound) { return false; }
        void updateSoundPosition(int sound, v3f pos) {}

diff --git a/src/client/sound_openal.cpp b/src/client/sound_openal.cpp
index 0eda8842be7266383ac5d16b3215d321cd0f3bb9..b015aba25ed9195afb5047f5bc48f6e951807129 100644 (file)
--- a/src/client/sound_openal.cpp
+++ b/src/client/sound_openal.cpp
@@ -322,7 +322,7 @@ class OpenALSoundManager: public ISoundManager
        OnDemandSoundFetcher *m_fetcher;
        ALCdevice *m_device;
        ALCcontext *m_context;
-       int m_next_id;
+       u16 m_last_used_id = 0; // only access within getFreeId() !
        std::unordered_map<std::string, std::vector<SoundBuffer*>> m_buffers;
        std::unordered_map<int, PlayingSound*> m_sounds_playing;
        struct FadeState {
@@ -342,8 +342,7 @@ class OpenALSoundManager: public ISoundManager
        OpenALSoundManager(SoundManagerSingleton *smg, OnDemandSoundFetcher *fetcher):
                m_fetcher(fetcher),
                m_device(smg->m_device.get()),
-               m_context(smg->m_context.get()),
-               m_next_id(1)
+               m_context(smg->m_context.get())
        {
                infostream << "Audio: Initialized: OpenAL " << std::endl;
        }
@@ -379,6 +378,22 @@ class OpenALSoundManager: public ISoundManager
                infostream << "Audio: Deinitialized." << std::endl;
        }
 
+       u16 getFreeId()
+       {
+               u16 startid = m_last_used_id;
+               while (!isFreeId(++m_last_used_id)) {
+                       if (m_last_used_id == startid)
+                               return 0;
+               }
+
+               return m_last_used_id;
+       }
+
+       inline bool isFreeId(int id) const
+       {
+               return id > 0 && m_sounds_playing.find(id) == m_sounds_playing.end();
+       }
+
        void step(float dtime)
        {
                doFades(dtime);
@@ -437,7 +452,7 @@ class OpenALSoundManager: public ISoundManager
                                << std::endl;
                assert(buf);
                PlayingSound *sound = new PlayingSound;
-               assert(sound);
+
                warn_if_error(alGetError(), "before createPlayingSoundAt");
                alGenSources(1, &sound->source_id);
                alSourcei(sound->source_id, AL_BUFFER, buf->buffer_id);
@@ -463,28 +478,17 @@ class OpenALSoundManager: public ISoundManager
        {
                assert(buf);
                PlayingSound *sound = createPlayingSound(buf, loop, volume, pitch);
-               if(!sound)
+               if (!sound)
                        return -1;
-               int id = m_next_id++;
-               m_sounds_playing[id] = sound;
-               return id;
-       }
 
-       int playSoundRawAt(SoundBuffer *buf, bool loop, float volume, const v3f &pos,
-                       float pitch)
-       {
-               assert(buf);
-               PlayingSound *sound = createPlayingSoundAt(buf, loop, volume, pos, pitch);
-               if(!sound)
-                       return -1;
-               int id = m_next_id++;
-               m_sounds_playing[id] = sound;
-               return id;
+               int handle = getFreeId();
+               m_sounds_playing[handle] = sound;
+               return handle;
        }
 
        void deleteSound(int id)
        {
-               std::unordered_map<int, PlayingSound*>::iterator i = m_sounds_playing.find(id);
+               auto i = m_sounds_playing.find(id);
                if(i == m_sounds_playing.end())
                        return;
                PlayingSound *sound = i->second;
@@ -580,39 +584,46 @@ class OpenALSoundManager: public ISoundManager
                alListenerf(AL_GAIN, gain);
        }
 
-       int playSound(const std::string &name, bool loop, float volume, float fade, float pitch)
+       int playSound(const SimpleSoundSpec &spec)
        {
                maintain();
-               if (name.empty())
+               if (spec.name.empty())
                        return 0;
-               SoundBuffer *buf = getFetchBuffer(name);
+               SoundBuffer *buf = getFetchBuffer(spec.name);
                if(!buf){
-                       infostream << "OpenALSoundManager: \"" << name << "\" not found."
+                       infostream << "OpenALSoundManager: \"" << spec.name << "\" not found."
                                        << std::endl;
                        return -1;
                }
+
                int handle = -1;
-               if (fade > 0) {
-                       handle = playSoundRaw(buf, loop, 0.0f, pitch);
-                       fadeSound(handle, fade, volume);
+               if (spec.fade > 0) {
+                       handle = playSoundRaw(buf, spec.loop, 0.0f, spec.pitch);
+                       fadeSound(handle, spec.fade, spec.gain);
                } else {
-                       handle = playSoundRaw(buf, loop, volume, pitch);
+                       handle = playSoundRaw(buf, spec.loop, spec.gain, spec.pitch);
                }
                return handle;
        }
 
-       int playSoundAt(const std::string &name, bool loop, float volume, v3f pos, float pitch)
+       int playSoundAt(const SimpleSoundSpec &spec, const v3f &pos)
        {
                maintain();
-               if (name.empty())
+               if (spec.name.empty())
                        return 0;
-               SoundBuffer *buf = getFetchBuffer(name);
-               if(!buf){
-                       infostream << "OpenALSoundManager: \"" << name << "\" not found."
+               SoundBuffer *buf = getFetchBuffer(spec.name);
+               if (!buf) {
+                       infostream << "OpenALSoundManager: \"" << spec.name << "\" not found."
                                        << std::endl;
                        return -1;
                }
-               return playSoundRawAt(buf, loop, volume, pos, pitch);
+
+               PlayingSound *sound = createPlayingSoundAt(buf, spec.loop, spec.gain, pos, spec.pitch);
+               if (!sound)
+                       return -1;
+               int handle = getFreeId();
+               m_sounds_playing[handle] = sound;
+               return handle;
        }
 
        void stopSound(int sound)
@@ -624,8 +635,9 @@ class OpenALSoundManager: public ISoundManager
        void fadeSound(int soundid, float step, float gain)
        {
                // Ignore the command if step isn't valid.
-               if (step == 0)
+               if (step == 0 || soundid < 0)
                        return;
+
                float current_gain = getSoundGain(soundid);
                step = gain - current_gain > 0 ? abs(step) : -abs(step);
                if (m_sounds_fading.find(soundid) != m_sounds_fading.end()) {

diff --git a/src/gui/guiFormSpecMenu.cpp b/src/gui/guiFormSpecMenu.cpp
index e4ea9ff58c37fd4d34d212118336cd531db62812..342f74b1687d2fb134218ab96a9967d21e91524f 100644 (file)
--- a/src/gui/guiFormSpecMenu.cpp
+++ b/src/gui/guiFormSpecMenu.cpp
@@ -4498,7 +4498,7 @@ bool GUIFormSpecMenu::OnEvent(const SEvent& event)
                                if ((s.ftype == f_TabHeader) &&
                                                (s.fid == event.GUIEvent.Caller->getID())) {
                                        if (!s.sound.empty() && m_sound_manager)
-                                               m_sound_manager->playSound(s.sound, false, 1.0f);
+                                               m_sound_manager->playSound(SimpleSoundSpec(s.sound, false, 1.0f));
                                        s.send = true;
                                        acceptInput();
                                        s.send = false;
@@ -4543,7 +4543,7 @@ bool GUIFormSpecMenu::OnEvent(const SEvent& event)
 
                                if (s.ftype == f_Button || s.ftype == f_CheckBox) {
                                        if (!s.sound.empty() && m_sound_manager)
-                                               m_sound_manager->playSound(s.sound, false, 1.0f);
+                                               m_sound_manager->playSound(SimpleSoundSpec(s.sound, false, 1.0f));
 
                                        s.send = true;
                                        if (s.is_exit) {
@@ -4568,7 +4568,7 @@ bool GUIFormSpecMenu::OnEvent(const SEvent& event)
                                                }
                                        }
                                        if (!s.sound.empty() && m_sound_manager)
-                                               m_sound_manager->playSound(s.sound, false, 1.0f);
+                                               m_sound_manager->playSound(SimpleSoundSpec(s.sound, false, 1.0f));
                                        s.send = true;
                                        acceptInput(quit_mode_no);
 
@@ -4586,7 +4586,7 @@ bool GUIFormSpecMenu::OnEvent(const SEvent& event)
                                        s.fdefault = L"";
                                } else if (s.ftype == f_Unknown || s.ftype == f_HyperText) {
                                        if (!s.sound.empty() && m_sound_manager)
-                                               m_sound_manager->playSound(s.sound, false, 1.0f);
+                                               m_sound_manager->playSound(SimpleSoundSpec(s.sound, false, 1.0f));
                                        s.send = true;
                                        acceptInput();
                                        s.send = false;

diff --git a/src/network/clientpackethandler.cpp b/src/network/clientpackethandler.cpp
index fba0fe72d91028419b441a62532000f45ef87a58..1901c66754d5b8d106bdd75deffe99332ed3fe81 100644 (file)
--- a/src/network/clientpackethandler.cpp
+++ b/src/network/clientpackethandler.cpp
@@ -820,12 +820,12 @@ void Client::handleCommand_PlaySound(NetworkPacket* pkt)
        s32 server_id;
 
        SimpleSoundSpec spec;
-       u8 type; // 0=local, 1=positional, 2=object
+       SoundLocation type; // 0=local, 1=positional, 2=object
        v3f pos;
        u16 object_id;
        bool ephemeral = false;
 
-       *pkt >> server_id >> spec.name >> spec.gain >> type >> pos >> object_id >> spec.loop;
+       *pkt >> server_id >> spec.name >> spec.gain >> (u8 &)type >> pos >> object_id >> spec.loop;
 
        try {
                *pkt >> spec.fade;
@@ -836,22 +836,20 @@ void Client::handleCommand_PlaySound(NetworkPacket* pkt)
        // Start playing
        int client_id = -1;
        switch(type) {
-               case 0: // local
-                       client_id = m_sound->playSound(spec);
-                       break;
-               case 1: // positional
-                       client_id = m_sound->playSoundAt(spec, pos);
-                       break;
-               case 2:
-               { // object
+       case SoundLocation::Local:
+               client_id = m_sound->playSound(spec);
+               break;
+       case SoundLocation::Position:
+               client_id = m_sound->playSoundAt(spec, pos);
+               break;
+       case SoundLocation::Object:
+               {
                        ClientActiveObject *cao = m_env.getActiveObject(object_id);
                        if (cao)
                                pos = cao->getPosition();
                        client_id = m_sound->playSoundAt(spec, pos);
                        break;
                }
-               default:
-                       break;
        }
 
        if (client_id != -1) {

diff --git a/src/script/common/c_content.cpp b/src/script/common/c_content.cpp
index 1f9f977817c83b37ad805444c634bd4f826b2688..5595f3b14c1d4ee3da02b7149a1dce5345564ec8 100644 (file)
--- a/src/script/common/c_content.cpp
+++ b/src/script/common/c_content.cpp
@@ -1056,7 +1056,7 @@ void read_server_sound_params(lua_State *L, int index,
                if(!lua_isnil(L, -1)){
                        v3f p = read_v3f(L, -1)*BS;
                        params.pos = p;
-                       params.type = ServerPlayingSound::SSP_POSITIONAL;
+                       params.type = SoundLocation::Position;
                }
                lua_pop(L, 1);
                lua_getfield(L, index, "object");
@@ -1065,7 +1065,7 @@ void read_server_sound_params(lua_State *L, int index,
                        ServerActiveObject *sao = ObjectRef::getobject(ref);
                        if(sao){
                                params.object = sao->getId();
-                               params.type = ServerPlayingSound::SSP_OBJECT;
+                               params.type = SoundLocation::Object;
                        }
                }
                lua_pop(L, 1);

diff --git a/src/script/lua_api/l_client.cpp b/src/script/lua_api/l_client.cpp
index aaced7cd0a09ab8343683ff5632dbf9573f35318..05ac53cbbe4ff6049e3097a1174a49f24e02722f 100644 (file)
--- a/src/script/lua_api/l_client.cpp
+++ b/src/script/lua_api/l_client.cpp
@@ -268,30 +268,32 @@ int ModApiClient::l_sound_play(lua_State *L)
        SimpleSoundSpec spec;
        read_soundspec(L, 1, spec);
 
+       SoundLocation type = SoundLocation::Local;
        float gain = 1.0f;
-       float pitch = 1.0f;
-       bool looped = false;
-       s32 handle;
+       v3f position;
 
        if (lua_istable(L, 2)) {
                getfloatfield(L, 2, "gain", gain);
-               getfloatfield(L, 2, "pitch", pitch);
-               getboolfield(L, 2, "loop", looped);
+               getfloatfield(L, 2, "pitch", spec.pitch);
+               getboolfield(L, 2, "loop", spec.loop);
 
                lua_getfield(L, 2, "pos");
                if (!lua_isnil(L, -1)) {
-                       v3f pos = read_v3f(L, -1) * BS;
+                       position = read_v3f(L, -1) * BS;
+                       type = SoundLocation::Position;
                        lua_pop(L, 1);
-                       handle = sound->playSoundAt(
-                                       spec.name, looped, gain * spec.gain, pos, pitch);
-                       lua_pushinteger(L, handle);
-                       return 1;
                }
        }
 
-       handle = sound->playSound(spec.name, looped, gain * spec.gain, spec.fade, pitch);
-       lua_pushinteger(L, handle);
+       spec.gain *= gain;
 
+       s32 handle;
+       if (type == SoundLocation::Local)
+               handle = sound->playSound(spec);
+       else
+               handle = sound->playSoundAt(spec, position);
+
+       lua_pushinteger(L, handle);
        return 1;
 }
 

diff --git a/src/server.cpp b/src/server.cpp
index 24e3526584bf3f810323ac5aa5cea57e3420778a..4eee14a301ac03581772eb3b36276ad9763b493a 100644 (file)
--- a/src/server.cpp
+++ b/src/server.cpp
@@ -138,21 +138,27 @@ void *ServerThread::run()
 
 v3f ServerPlayingSound::getPos(ServerEnvironment *env, bool *pos_exists) const
 {
-       if(pos_exists) *pos_exists = false;
-       switch(type){
-       case SSP_LOCAL:
+       if (pos_exists)
+               *pos_exists = false;
+
+       switch (type ){
+       case SoundLocation::Local:
                return v3f(0,0,0);
-       case SSP_POSITIONAL:
-               if(pos_exists) *pos_exists = true;
+       case SoundLocation::Position:
+               if (pos_exists)
+                       *pos_exists = true;
                return pos;
-       case SSP_OBJECT: {
-               if(object == 0)
-                       return v3f(0,0,0);
-               ServerActiveObject *sao = env->getActiveObject(object);
-               if(!sao)
-                       return v3f(0,0,0);
-               if(pos_exists) *pos_exists = true;
-               return sao->getBasePosition(); }
+       case SoundLocation::Object:
+               {
+                       if (object == 0)
+                               return v3f(0,0,0);
+                       ServerActiveObject *sao = env->getActiveObject(object);
+                       if (!sao)
+                               return v3f(0,0,0);
+                       if (pos_exists)
+                               *pos_exists = true;
+                       return sao->getBasePosition();
+               }
        }
        return v3f(0,0,0);
 }
@@ -2071,9 +2077,9 @@ s32 Server::playSound(ServerPlayingSound &params, bool ephemeral)
 {
        // Find out initial position of sound
        bool pos_exists = false;
-       v3f pos = params.getPos(m_env, &pos_exists);
+       const v3f pos = params.getPos(m_env, &pos_exists);
        // If position is not found while it should be, cancel sound
-       if(pos_exists != (params.type != ServerPlayingSound::SSP_LOCAL))
+       if(pos_exists != (params.type != SoundLocation::Local))
                return -1;
 
        // Filter destination clients

diff --git a/src/server.h b/src/server.h
index 00b6f897dface3366fd0bb24d6c2eee4174b771a..cb7d77067d8fde8e5f6cc94ac7626ec29119a7c1 100644 (file)
--- a/src/server.h
+++ b/src/server.h
@@ -99,11 +99,7 @@ struct MediaInfo
 // Combines the pure sound (SimpleSoundSpec) with positional information
 struct ServerPlayingSound
 {
-       enum Type {
-               SSP_LOCAL,
-               SSP_POSITIONAL,
-               SSP_OBJECT
-       } type = SSP_LOCAL;
+       SoundLocation type = SoundLocation::Local;
 
        float gain = 1.0f; // for amplification of the base sound
        float max_hear_distance = 32 * BS;

diff --git a/src/sound.h b/src/sound.h
index ddb4e3dc2dcb69c518b3915433c93baa8efeb8e6..801c552a96143d6dcb5c7d950a5040f9e7da0b82 100644 (file)
--- a/src/sound.h
+++ b/src/sound.h
@@ -59,3 +59,11 @@ struct SimpleSoundSpec
        float pitch = 1.0f;
        bool loop = false;
 };
+
+
+// The order must not be changed. This is sent over the network.
+enum class SoundLocation : u8 {
+       Local,
+       Position,
+       Object
+};