Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lua API: Add sound_pause and sound_resume to minetest api #5124

Closed
wants to merge 33 commits into from

Conversation

@14NGiestas
Copy link
Contributor

commented Jan 28, 2017

Hello folks!
I'm learning C++ so I decided to practice adding this to minetest lua api... My first issue in this repo was about adding a range option in minetest.sound_play, however I realized (after all this time) that a range option isn't needed to solve my initial problem.
I'm so happy I did something barely useful in C++ in a huge project... after many tries, energy drinks and a while reading the docs of OpenAL and Minetest core stuff... searching in a bunch of files 馃槳
Well... whatever, let's show the stuff:

Exemple use (dunno why I used a "c" table):

c = {}
c.id = 0

minetest.register_chatcommand("play", {
	params = "<text>",
	description = "playsound",
	func = function( _ , text)
		c.id = minetest.sound_play("teste_q") -- Nothing new here uh?
		print(c.id)
	end,
})

minetest.register_chatcommand("pause", {
	description = "pause sound",
	func = function( _ )
		minetest.sound_pause(c.id) -- This pause the sound
	end,
})

minetest.register_chatcommand("resume", {
	description = "resume sound",
	func = function( _ )
		minetest.sound_resume(c.id) -- This resume the paused sound
	end,
})

minetest.register_chatcommand("stop", {
	description = "stop sound",
	func = function( _ )
		minetest.sound_stop(c.id) -- This completely stop the sound
	end,
})
PlayingSound *sound = i->second;
alSourcePause(sound->source_id);
}
void resumeSnd(int id)

This comment has been minimized.

Copy link
@nerzhul

nerzhul Jan 28, 2017

Member

missing empty line

void resumeSnd(int id)
{
UNORDERED_MAP<int, PlayingSound*>::iterator i = m_sounds_playing.find(id);
if(i == m_sounds_playing.end())

This comment has been minimized.

Copy link
@nerzhul

nerzhul Jan 28, 2017

Member

missing space

maintain();
pauseSnd(sound);
}
void resumeSound(int sound)

This comment has been minimized.

Copy link
@nerzhul

nerzhul Jan 28, 2017

Member

missing empty line

if(state != AL_PLAYING){
del_list.insert(id);
if(state != AL_PLAYING && state != AL_PAUSED)
{

This comment has been minimized.

Copy link
@nerzhul

nerzhul Jan 28, 2017

Member

code style problem, brace + space

@@ -108,8 +108,8 @@ const ToClientCommandHandler toClientCommandTable[TOCLIENT_NUM_MSG_TYPES] =
{ "TOCLIENT_LOCAL_PLAYER_ANIMATIONS", TOCLIENT_STATE_CONNECTED, &Client::handleCommand_LocalPlayerAnimations }, // 0x51
{ "TOCLIENT_EYE_OFFSET", TOCLIENT_STATE_CONNECTED, &Client::handleCommand_EyeOffset }, // 0x52
{ "TOCLIENT_DELETE_PARTICLESPAWNER", TOCLIENT_STATE_CONNECTED, &Client::handleCommand_DeleteParticleSpawner }, // 0x53
null_command_handler,
null_command_handler,
{ "TOCLIENT_PAUSE_SOUND", TOCLIENT_STATE_CONNECTED, &Client::handleCommand_PauseSound }, // 0x54

This comment has been minimized.

Copy link
@nerzhul

nerzhul Jan 28, 2017

Member

you forgot sending rule into serveropcodes.cpp

This comment has been minimized.

Copy link
@14NGiestas

14NGiestas Jan 31, 2017

Author Contributor

Done! Thank you!
@nerzhul but this comment is in the wrong file isn't?

@@ -388,6 +388,8 @@ class Client : public con::PeerHandler, public InventoryManager, public IGameDef
void handleCommand_ItemDef(NetworkPacket* pkt);
void handleCommand_PlaySound(NetworkPacket* pkt);
void handleCommand_StopSound(NetworkPacket* pkt);
void handleCommand_PauseSound(NetworkPacket* pkt);
void handleCommand_ResumeSound(NetworkPacket* pkt);

This comment has been minimized.

Copy link
@nerzhul

nerzhul Jan 28, 2017

Member

wrong indent

@@ -429,6 +429,7 @@ int ModApiServer::l_sound_play(lua_State *L)
return 1;
}


This comment has been minimized.

Copy link
@nerzhul

nerzhul Jan 28, 2017

Member

useless

@paramat

This comment has been minimized.

Copy link
Member

commented Jan 28, 2017

What are the uses for these?

@rubenwardy

This comment has been minimized.

Copy link
Member

commented Jan 28, 2017

Stopping long or looping music, pretty much.
Ie: juke boxes

@@ -491,7 +511,8 @@ class OpenALSoundManager: public ISoundManager
{
ALint state;
alGetSourcei(sound->source_id, AL_SOURCE_STATE, &state);
if(state != AL_PLAYING){
if(state != AL_PLAYING && state != AL_PAUSED)
{

This comment has been minimized.

Copy link
@nerzhul

nerzhul Jan 31, 2017

Member

brace on wrong line

@@ -511,8 +511,7 @@ class OpenALSoundManager: public ISoundManager
{
ALint state;
alGetSourcei(sound->source_id, AL_SOURCE_STATE, &state);
if(state != AL_PLAYING && state != AL_PAUSED)
{
if(state != AL_PLAYING && state != AL_PAUSED) {

This comment has been minimized.

Copy link
@nerzhul

nerzhul Jan 31, 2017

Member

missing space after if, please read our code style guidelines

This comment has been minimized.

Copy link
@14NGiestas

14NGiestas Jan 31, 2017

Author Contributor

Oh sorry about that @nerzhul I'm really new here.
I'm reading the code guidelines now (What a shame).
I hope there's no more heresies left.


*pkt >> server_id;

UNORDERED_MAP<s32, int>::iterator i = m_sounds_server_to_client.find(server_id);

This comment has been minimized.

Copy link
@rubenwardy

rubenwardy Jan 31, 2017

Member

please use it as it's an iterator not a counter (i)

This comment has been minimized.

Copy link
@14NGiestas

14NGiestas Jan 31, 2017

Author Contributor

@rubenwardy this one is the same as every original iterator name in the minetest sound api stuff.
If this is inaccurate, should I change every iterator I've seen labeled as i (like in some functions above this one)?

14NGiestas added 2 commits Feb 1, 2017
@14NGiestas

This comment has been minimized.

Copy link
Contributor Author

commented Feb 16, 2017

Any issue left?

@theunifiedmind7

This comment has been minimized.

Copy link

commented Feb 17, 2017

Jukeboxes would work much better if you could get the length of the sound file...

EDIT: Don't forget about documentation in lua_api.txt

@rubenwardy
Copy link
Member

left a comment

Looks okay apart from these style issues (and missing docs), I'd need to look and understand the code more however - and test

void pauseSnd(int id)
{
UNORDERED_MAP<int, PlayingSound*>::iterator i = m_sounds_playing.find(id);
if(i == m_sounds_playing.end())

This comment has been minimized.

Copy link
@rubenwardy
@@ -454,6 +454,26 @@ class OpenALSoundManager: public ISoundManager
m_sounds_playing.erase(id);
}

void pauseSnd(int id)
{
UNORDERED_MAP<int, PlayingSound*>::iterator i = m_sounds_playing.find(id);

This comment has been minimized.

Copy link
@rubenwardy

void resumeSnd(int id)
{
UNORDERED_MAP<int, PlayingSound*>::iterator i = m_sounds_playing.find(id);

This comment has been minimized.

Copy link
@rubenwardy
@@ -491,7 +511,8 @@ class OpenALSoundManager: public ISoundManager
{
ALint state;
alGetSourcei(sound->source_id, AL_SOURCE_STATE, &state);
if(state != AL_PLAYING){

if(state != AL_PLAYING && state != AL_PAUSED) {

This comment has been minimized.

Copy link
@rubenwardy
@@ -331,7 +331,9 @@ class Client : public con::PeerHandler, public InventoryManager, public IGameDef
void handleCommand_ItemDef(NetworkPacket* pkt);
void handleCommand_PlaySound(NetworkPacket* pkt);
void handleCommand_StopSound(NetworkPacket* pkt);
void handleCommand_FadeSound(NetworkPacket *pkt);
void handleCommand_PauseSound(NetworkPacket* pkt);

This comment has been minimized.

Copy link
@Dumbeldor

Dumbeldor Jul 4, 2017

Contributor

void handleCommand_PauseSound(NetworkPacket *pkt);

@@ -331,7 +331,9 @@ class Client : public con::PeerHandler, public InventoryManager, public IGameDef
void handleCommand_ItemDef(NetworkPacket* pkt);
void handleCommand_PlaySound(NetworkPacket* pkt);
void handleCommand_StopSound(NetworkPacket* pkt);
void handleCommand_FadeSound(NetworkPacket *pkt);
void handleCommand_PauseSound(NetworkPacket* pkt);
void handleCommand_ResumeSound(NetworkPacket* pkt);

This comment has been minimized.

Copy link
@Dumbeldor

Dumbeldor Jul 4, 2017

Contributor

void handleCommand_PauseSound(NetworkPacket *pkt);

std::unordered_map<s32, int>::iterator i = m_sounds_server_to_client.find(server_id);
if (i != m_sounds_server_to_client.end()) {
int client_id = i->second;
std::unordered_map<s32, int>::iterator it = m_sounds_server_to_client.find(server_id);

This comment has been minimized.

Copy link
@Dumbeldor

Dumbeldor Jul 4, 2017

Contributor

const std::unordered_map


*pkt >> server_id;

std::unordered_map<s32, int>::iterator it = m_sounds_server_to_client.find(server_id);

This comment has been minimized.

Copy link
@Dumbeldor

Dumbeldor Jul 4, 2017

Contributor

const std::unordered_map

@@ -159,7 +159,7 @@ SoundBuffer *load_opened_ogg_file(OggVorbis_File *oggFile,

ALenum error = alGetError();

if(error != AL_NO_ERROR){
if (error != AL_NO_ERROR){

This comment has been minimized.

Copy link
@Dumbeldor

Dumbeldor Jul 4, 2017

Contributor

add space before {


alDeleteSources(1, &sound->source_id);

delete sound;
m_sounds_playing.erase(id);
}

void pauseSnd(int id)
{
std::unordered_map<int, PlayingSound*>::iterator it = m_sounds_playing.find(id);

This comment has been minimized.

Copy link
@Dumbeldor

Dumbeldor Jul 4, 2017

Contributor

const std::unordered_map
and
PlayingSound *>

@@ -384,11 +384,11 @@ class OpenALSoundManager: public ISoundManager

SoundBuffer* getBuffer(const std::string &name)
{
std::unordered_map<std::string, std::vector<SoundBuffer*>>::iterator i =
std::unordered_map<std::string, std::vector<SoundBuffer*> >::iterator it =

This comment has been minimized.

Copy link
@Dumbeldor

Dumbeldor Jul 4, 2017

Contributor

SoundBuffer *>

@@ -384,11 +384,11 @@ class OpenALSoundManager: public ISoundManager

SoundBuffer* getBuffer(const std::string &name)

This comment has been minimized.

Copy link
@Dumbeldor

Dumbeldor Jul 4, 2017

Contributor

SoundBuffer *getBuffer

@@ -370,10 +370,10 @@ class OpenALSoundManager: public ISoundManager

void addBuffer(const std::string &name, SoundBuffer *buf)
{
std::unordered_map<std::string, std::vector<SoundBuffer*>>::iterator i =
std::unordered_map<std::string, std::vector<SoundBuffer*> >::iterator it =

This comment has been minimized.

Copy link
@Dumbeldor

Dumbeldor Jul 4, 2017

Contributor

<SoundBuffer *>

void updateSoundPosition(int id, v3f pos)
{
std::unordered_map<int, PlayingSound*>::iterator i = m_sounds_playing.find(id);
if (i == m_sounds_playing.end())
std::unordered_map<int, PlayingSound*>::iterator it = m_sounds_playing.find(id);

This comment has been minimized.

Copy link
@Dumbeldor

Dumbeldor Jul 4, 2017

Contributor

const std::unordered_map

@rubenwardy

This comment has been minimized.

Copy link
Member

commented Dec 4, 2017

Still interested in working on this?

@paramat

This comment has been minimized.

Copy link
Member

commented Dec 4, 2017

Note we can close PRs if no author response in a month.
@14NGiestas
I'd like to see some need for this first, it seems of little use.

@14NGiestas

This comment has been minimized.

Copy link
Contributor Author

commented Dec 4, 2017

@paramat

This comment has been minimized.

Copy link
Member

commented Dec 7, 2017

Yes fine, add a comment in this thread if or when you want it re-opened.

@paramat paramat closed this Dec 7, 2017

@paramat paramat added WIP and removed Adoption needed labels Dec 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can鈥檛 perform that action at this time.