ravachol / kew

A command-line music player
GNU General Public License v2.0
575 stars 22 forks source link

Problems with the gotoSong function #57

Closed asientador closed 1 year ago

asientador commented 1 year ago

Hey! I need some advice on the gotoSong function. I don't really know how to get an integer from the user while the main loop is still running.

ravachol commented 1 year ago

Yeah, it's not made for taking input like that.

Does the user type in a number and then press enter? So you'd have to detect the first number, then start listening for more numbers until you get an Enter or maybe escape to cancel. The music will continue playing since it's in a different thread. the problem is the player text/ui. It would not update until you get an enter. You could solve that by having a global array numbersPressed and processInput() would just add to that in case of numbers until an enter was pressed. you could cancel the whole thing as soon as any key except numbers or enter was pressed.

pseudo code: if (isanumber(keypressed)) add to numbersPressed Array else if (keypressed == enter && numbersPressed != NULL) event.type = goto_song. else numbersPressed = NULL switch (event.key) { / do the usual stuff / }

something like that.

asientador commented 1 year ago

Thanks a lot!. Seems like a really good way to implement the functionality. I'll give it a try.

ravachol commented 1 year ago

btw, check your email.

asientador commented 1 year ago

Not finished yet. Still working on it!.

ravachol commented 1 year ago

ok, cool. I'll have a look soon. The thing I can say right away is that term.c is supposed to be a file with just utility functions, it should know nothing about cue. same goes for stringfunc.c, file.c and cache.c. I should probably add a comment about that at the top of those files.

asientador commented 1 year ago

Need some help, I actually made it to change the song list and go to the song. But the player doesn't update, and the current song is still playing. Code is out there on my fork.

ravachol commented 1 year ago

I left a comment here, but realized it was incorrect. apart from this part

pseudo: Node* getSongByNumber(songNumber) { song = playlist.head int count = 1; while (song.next != null) { if (count == songNumber) break } count++; song = song->next: } return song;

}

asientador commented 1 year ago

I'll try to re-code this but following your instructions. Organizing the files and making each function wherever it should be. Thanks a lot by the way.

ravachol commented 1 year ago

you're welcome, I have to take a better look. I think you need more help, because it's not easy.

ravachol commented 1 year ago

I think the cleanest way, would be if you made a function almost exactly like skiptoprevsong.. but instead void skiptonumberedsong(int songnumber).

void skipToNumberedSong(int songNumber) { if (songLoading || !loadedNextSong || skipping) return; skipping = true; loadedNextSong = false; songLoading = true;

currentSong = getSongbyNumber(songNumber); 
if (usingSongDataA)
{
    loadingdata.loadA = false;
    unloadSongData(&loadingdata.songdataB);
}
else
{
    loadingdata.loadA = true;
    unloadSongData(&loadingdata.songdataA);
}

loadSong(currentSong, &loadingdata);
while (!loadedNextSong && !loadingFailed)
{
    usleep(10000);
}
clock_gettime(CLOCK_MONOTONIC, &start_time);   
skip();

} `

then you need to handle it in preparenextsong:

if (!skipPrev && !gotoSong && !repeatEnabled) currentSong = currentSong->next; else { skipPrev = false; gotosong = false; }

asientador commented 1 year ago

Your advice and function worked flawlesly. I moved the code to fit the project structure. The gotoSong function is now working like a charm. Not pushing the changes yet because I need to fix some bugs.

By the way, thanks for your help! You gave me the push I needed.

ravachol commented 1 year ago

solution looks good but there are some adjustments that could/should be made.

char digitsPressed[10];

if (isdigit) { digitsPressed[digitsPressedCount++] = event.key; } else { switch(event.key) { case '\n' // ENTER if (digitsPressedCount > 0) event.type = event_gotosong // more keys... } }

later in handle input:

case EVENT_GOTOSONG: gotosong = true; refresh = true; digitsPressedCount = 0; digitsPressed[0] = '\0'; skipToNumberedSong();

something like this anyway...cheers!

oh..and also reset digitsPressed and digitsPressedCount whenever user presses something that isn't a number

asientador commented 1 year ago

Yeah, that sounds really good! I'll change that as soon as I fix a bug that I'm having trouble with. Changing song a second time in the same execution gives a segmentation fault. I think that the problem has to do with the playlist not being updated.

ravachol commented 1 year ago

the playlist is just a list, the only thing that gets updated is this guy:

Node *currentSong = NULL;

asientador commented 1 year ago

Hey @ravachol I need some help before I fix the problems you told me before. When you change to a song by its number it's actually working fine, but if u want to change to another song from there it crash. Tried a bunch of things but don't really get to the roots of the problem.

ravachol commented 1 year ago

ok, I'll have a look.

ravachol commented 1 year ago

I sent you some updated files via email. I think I found the bug, mainly this was happening:

currentSong = getSongByNumber(songNumber);
if (usingSongDataA)
{
    loadingdata.loadA = false;
    unloadSongData(&loadingdata.songdataB);
}
else
{
    loadingdata.loadA = true;
    unloadSongData(&loadingdata.songdataA);
}

both of those were false which meant it wasn't loading songs as it's supposed to.

asientador commented 1 year ago

Thanks a lot! I'll take a look at it this afternoon.